[MBI] V8TaskRunner Audit. Can it run arbitrary JavaScript?

[Minoru Chikamune]

Hi,

A Chrome renderer process has a process-global V8TaskRunner. It was unclear if the V8TaskRunner was purely for scheduling GC tasks, or if it could run arbitrary JavaScript code which can mutate per-ASG data.

After analysis, it turns out that V8TaskRunner can indeed run arbitrary JavaScript code via FinalizationRegistry API, suggesting that it should be migrated to per-AgentSchedulingGroup.

Please check the document below if you are interested.

https://docs.google.com/document/d/1KmCwYB7dcx4021aw4E7h9uYB1WlxhriGAd-mJzpTXQY/edit#heading=h.ufga5mrz8dlv

[Kentaro Hara]

Thanks Minoru-san for the doc!

Do you (or does anyone) know how the behavior of FinalizationRegistry callbacks is standardized? Per the spec, what task runner should be used?

I'm asking this because I'm not sure if it's realistic to run FinalizationRegistry callbacks in per-ASG task runners. If we want to do this, V8 needs to repost a task every time it invokes callbacks. The overhead won't be acceptable... Or am I missing something?

--
You received this message because you are subscribed to the Google Groups "Blink Isolation discussions" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-isolation...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-isolation-dev/CAL4s8FH9ruzbMej%3D7g6ok_OLX3220sDFEnesiFixpYdYcqHGrQ%40mail.gmail.com.

--

Kentaro Hara, Tokyo

[Michael Lippautz]

+s...@chromium.org, +ma...@chromium.org for FinalizationRegistry knowledge

On Thu, Feb 18, 2021 at 2:25 PM Kentaro Hara <har...@chromium.org> wrote:

Thanks Minoru-san for the doc!

Do you (or does anyone) know how the behavior of FinalizationRegistry callbacks is standardized? Per the spec, what task runner should be used?

I'm asking this because I'm not sure if it's realistic to run FinalizationRegistry callbacks in per-ASG task runners. If we want to do this, V8 needs to repost a task every time it invokes callbacks. The overhead won't be acceptable... Or am I missing something?

IIUC, callbacks are collected and there's one task per GC cycle that dispatches all of the FinalizationRegistry cleanups in their respective contexts.

 

On Thu, Feb 18, 2021 at 9:56 PM 'Minoru Chikamune' via Blink Isolation discussions <blink-iso...@chromium.org> wrote:

Hi,

A Chrome renderer process has a process-global V8TaskRunner. It was unclear if the V8TaskRunner was purely for scheduling GC tasks, or if it could run arbitrary JavaScript code which can mutate per-ASG data.

After analysis, it turns out that V8TaskRunner can indeed run arbitrary JavaScript code via FinalizationRegistry API, suggesting that it should be migrated to per-AgentSchedulingGroup.

Please check the document below if you are interested.

https://docs.google.com/document/d/1KmCwYB7dcx4021aw4E7h9uYB1WlxhriGAd-mJzpTXQY/edit#heading=h.ufga5mrz8dlv

--
You received this message because you are subscribed to the Google Groups "Blink Isolation discussions" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-isolation...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-isolation-dev/CAL4s8FH9ruzbMej%3D7g6ok_OLX3220sDFEnesiFixpYdYcqHGrQ%40mail.gmail.com.

--

Kentaro Hara, Tokyo

--
You received this message because you are subscribed to the Google Groups "Blink Isolation discussions" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-isolation...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-isolation-dev/CABg10jyeCoJC%2Bp39%3DHFVfbD8nZ3vwv%3Dh-X65U6hmN5Cs6%2BG4aA%40mail.gmail.com.

[Kentaro Hara]

IIUC, callbacks are collected and there's one task per GC cycle that dispatches all of the FinalizationRegistry cleanups in their respective contexts.

That matches my understanding. V8 only enters v8::Context before invoking the callback.

If we want to make the callback use a correct per-ASG task runner, we need to repost a task per callback. This doesn't sound realistic to me.

I'm curious about the expected behavior in the spec :)

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-isolation-dev/CAH%2BmL5CcvCSq56q7_ZE%3DRUUVDCcEF_a17QrtnTOmLi-Hb-CV9w%40mail.gmail.com.

--

Kentaro Hara, Tokyo

[Shu-yu Guo]

Hi all,

The JS spec side of posting tasks (called a Job in JS spec-ese) is here. The HTML integration PR that describes task queuing behavior is here. We should get that PR merged, which was blocked on sorting out incumbent settings object stuff, which we finally did. The semantics:

• The task source is currently named the "garbage collection task source", but from this comment we will rename it to something like "JS engine task source" for all tasks posted by the JS engine and by features specified in ECMAScript. 
• One task is posted per FinalizationRegistry, not per callback invocation, not one task for all FinalizationRegistries. Each FinalizationRegistry may invoke its callback many times, on each dead cell, in a single task. These callbacks are called in a loop over some or all of the dead cells, and the correct v8::Context should be entered before entering into this loop.
• Note that the specced one-task-per-FinalizationRegistry behavior is not observable. Even though it looks like the spec posts separate tasks and the relative ordering of those tasks when interleaved with Atomics.waitAsync tasks on the same task source would be observable, the JS spec gives significant leeway for when these finalization tasks are posted. The JS engine can choose to post all FinalizationRegistry tasks back-to-back, which would be observably equivalent to coalescing tasks. That said it doesn't seem desirable to schedule them all within one task in case finalization takes too long.
  
• There is no spec guarantee that each task must run the FinalizationRegistry callback on all its dead cells. If a task doesn't want to process all dead cells in a FinalizationRegistry, it should requeue a task. This is to give implementations flexibility in budgeting time for these tasks.

[Shu-yu Guo]

I misspoke on one point above: the coalescing tasks behavior is observable from a microtask checkpoint perspective. If tasks are coalesced, then a manual microtask checkpoint must be performed between processing each individual FinalizationRegistry.

[Kouhei Ueno]

On Fri, Feb 19, 2021 at 3:15 AM Shu-yu Guo <s...@chromium.org> wrote:

I misspoke on one point above: the coalescing tasks behavior is observable from a microtask checkpoint perspective. If tasks are coalesced, then a manual microtask checkpoint must be performed between processing each individual FinalizationRegistry.

On Thu, Feb 18, 2021 at 10:12 AM Shu-yu Guo <s...@chromium.org> wrote:

Hi all,

The JS spec side of posting tasks (called a Job in JS spec-ese) is here. The HTML integration PR that describes task queuing behavior is here. We should get that PR merged, which was blocked on sorting out incumbent settings object stuff, which we finally did. The semantics:

• The task source is currently named the "garbage collection task source", but from this comment we will rename it to something like "JS engine task source" for all tasks posted by the JS engine and by features specified in ECMAScript. 
• One task is posted per FinalizationRegistry, not per callback invocation, not one task for all FinalizationRegistries. Each FinalizationRegistry may invoke its callback many times, on each dead cell, in a single task. These callbacks are called in a loop over some or all of the dead cells, and the correct v8::Context should be entered before entering into this loop.
• Note that the specced one-task-per-FinalizationRegistry behavior is not observable. Even though it looks like the spec posts separate tasks and the relative ordering of those tasks when interleaved with Atomics.waitAsync tasks on the same task source would be observable, the JS spec gives significant leeway for when these finalization tasks are posted. The JS engine can choose to post all FinalizationRegistry tasks back-to-back, which would be observably equivalent to coalescing tasks. That said it doesn't seem desirable to schedule them all within one task in case finalization takes too long.
  
• There is no spec guarantee that each task must run the FinalizationRegistry callback on all its dead cells. If a task doesn't want to process all dead cells in a FinalizationRegistry, it should requeue a task. This is to give implementations flexibility in budgeting time for these tasks.

On Thu, Feb 18, 2021 at 5:48 AM Kentaro Hara <har...@chromium.org> wrote:

IIUC, callbacks are collected and there's one task per GC cycle that dispatches all of the FinalizationRegistry cleanups in their respective contexts.

That matches my understanding. V8 only enters v8::Context before invoking the callback.

If we want to make the callback use a correct per-ASG task runner, we need to repost a task per callback. This doesn't sound realistic to me.

Would you elaborate on why? I think reposting tasks per FinalizationRegistry as syg@ mentioned is feasible. Then, we can change the postTask target to its associated ASG V8TaskRunner to ensure all user JavaScript runs on a per-ASG task runner.

 

You received this message because you are subscribed to the Google Groups "scheduler-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scheduler-de...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/scheduler-dev/CAN-e9e--k68EiyuNcgXHDJE%2BbXhz%3D8u7S6z1K9Ywk91TEk0N3w%40mail.gmail.com.

--

kouhei

[Kentaro Hara]

Would you elaborate on why? I think reposting tasks per FinalizationRegistry as syg@ mentioned is feasible. Then, we can change the postTask target to its associated ASG V8TaskRunner to ensure all user JavaScript runs on a per-ASG task runner.

You're right. If we can repost tasks per FinalizationRegistry, it will work. (I was thinking we need to repost tasks per callback, then the performance overhead will be too much.)

One more thing: Any C++ callback called from V8 may invoke any arbitrary JavaScript unless we are really careful. For example, if the C++ callback uses v8_object->Get() and the getter is overridden in the user script, it may invoke any arbitrary JavaScript.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-isolation-dev/CAPVAxLUybWxOrTNwtH8HHJ9Bg5fuZkYgG3Ph7o10xveWMcQ9XA%40mail.gmail.com.

--

Kentaro Hara, Tokyo