Hi all,
We recently ran into a crash in Chromium which was a result of a
'MicrotasksCompletedCallback' being called twice in one C++ stack.
The documentation for 'AddMicrotasksCompletedCallback' explicitly states
that executing scripts inside the callback will not re-trigger microtasks
and the callback, but that seems to no longer be the case.
'MicrotaskQueue::RunMicrotasks' has a 'SetIsRunningMicrotasks' that only
covers 'Execution::TryRunMicrotasks', not 'OnCompleted()'.
In our case, the reason for the re-entrancy and crash was as follows:
1. 'blink::V8ScriptRunner::CallFunction' created a 'MicrotasksScope'
object and called into script.
2. '~MicrotasksScope' noticed it was the only item on the stack
(decrementing its depth went to 0), so it executed microtasks.
3. 'MicrotaskQueue::OnCompleted' creates a copy of the callbacks, and
then executes each one in turn.
4. Our callback was triggered, which we use to look for unhandled
promise rejections. As part of discovering and handling an unhandled
promise rejection, we can end up calling into script.
5. When we call into script, we create a 'MicrotasksScope' object.
6. '~MicrotasksScope` for this nested scope again decremented the depth
to 0 (as the one from 2. above decremented the depth already).
7. We again ran microtasks.
8. 'MicrotaskQueue::OnCompleted' is called, and again copies all the
callbacks, then executes each one in turn.
9. Our callback this time has nothing to do, so we move onto the second
callback:
10. Blink's callback is called ('EventLoop::RunEndOfCheckpointTasks'),
and it attempts to remove the callback from the set of callbacks to call,
to prevent it from being called multiple times.
11. The stack unwinds back to 3. above, and it moves onto blinks
callback again, as it had already created a copy of the callbacks, so
removing the callback in 10. had no effect.
12. Blink's callback is called again, and as this code has 'AddRef()'
and 'Release()' calls that are supposed to come in pairs, the callback ends
up accessing an object that's already been released in 10.
Is the documentation on 'AddMicrotasksCompletedCallback' intended to still
be true? I believe this is the commit
<https://github.com/v8/v8/commit/ba363c755b14d8d81313509b797f421226bc4ea1#diff-a2c1a2ea95fdac618e585ba3e7b1a326cb014d3c0f196962e2dcc222a9fb9e5aR122>
that stopped it from being so.
Thanks,
Daryl.
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups
"v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/v8-dev/9bd8a0d9-d6af-4085-a8dc-8ccac7d186f8n%40googlegroups.com.
