Microtasks policy incorrect, leading to DCHECK, when calling window.open from setImmediate

[nornagon]

To repro, run this fiddle under a debug build: https://gist.github.com/b72948eafa5a2f61acf9f4718ff09259

#
# Fatal error in ../../v8/src/api/api-inl.h, line 183
# Debug check failed: microtask_queue->GetMicrotasksScopeDepth() || !microtask_queue->DebugMicrotasksScopeDepthIsZero().
#
#
#
#FailureMessage Object: 0x7ffeebb54620
0   Electron Framework                  0x000000010c472bb9 base::debug::CollectStackTrace(void**, unsigned long) + 9
1   Electron Framework                  0x000000010c38e243 base::debug::StackTrace::StackTrace() + 19
2   Electron Framework                  0x000000010e820ffd gin::(anonymous namespace)::PrintStackTrace() + 45
3   Electron Framework                  0x000000010e1f7136 V8_Fatal(char const*, int, char const*, ...) + 326
4   Electron Framework                  0x000000010e1f6b15 v8::base::(anonymous namespace)::DefaultDcheckHandler(char const*, int, char const*) + 21
5   Electron Framework                  0x0000000109695f11 v8::CallDepthScope<true>::~CallDepthScope() + 865
6   Electron Framework                  0x0000000109653283 v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) + 835
7   Electron Framework                  0x00000001118657e2 node::InternalMakeCallback(node::Environment*, v8::Local<v8::Object>, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) + 738
8   Electron Framework                  0x0000000111865aab node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) + 187
9   Electron Framework                  0x00000001118aebf5 node::Environment::CheckImmediate(uv_check_s*) + 181
10  Electron Framework                  0x0000000107c458ad uv__run_check + 157
11  Electron Framework                  0x0000000107c3f7e1 uv_run + 369
12  Electron Framework                  0x0000000107e0964c electron::NodeBindings::UvRunOnce() + 348
13  Electron Framework                  0x0000000107e0ae04 base::internal::Invoker<base::internal::BindState<void (electron::NodeBindings::*)(), base::WeakPtr<electron::NodeBindings> >, void ()>::RunOnce(base::internal::BindStateBase*) + 148
14  Electron Framework                  0x000000010c40a190 base::TaskAnnotator::RunTask(char const*, base::PendingTask*) + 400
15  Electron Framework                  0x000000010c42e292 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::sequence_manager::LazyNow*) + 994
16  Electron Framework                  0x000000010c42dab8 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 88
17  Electron Framework                  0x000000010c489d71 base::MessagePumpCFRunLoopBase::RunWork() + 65
18  Electron Framework                  0x000000010c4829a2 base::mac::CallWithEHFrame(void () block_pointer) + 10
19  Electron Framework                  0x000000010c48979f base::MessagePumpCFRunLoopBase::RunWorkSource(void*) + 63
20  CoreFoundation                      0x00007fff2f446832 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
21  CoreFoundation                      0x00007fff2f4467d1 __CFRunLoopDoSource0 + 103
22  CoreFoundation                      0x00007fff2f4465eb __CFRunLoopDoSources0 + 209
23  CoreFoundation                      0x00007fff2f44531a __CFRunLoopRun + 927
24  CoreFoundation                      0x00007fff2f44491e CFRunLoopRunSpecific + 462
25  Foundation                          0x00007fff31ae01a8 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 212
26  Electron Framework                  0x000000010c48a3ae base::MessagePumpNSRunLoop::DoRun(base::MessagePump::Delegate*) + 126
27  Electron Framework                  0x000000010c48914c base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 140
28  Electron Framework                  0x000000010c42ee5b base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) + 651
29  Electron Framework                  0x000000010c3e2d6a base::RunLoop::Run(base::Location const&) + 890
30  Electron Framework                  0x00000001115efc1b content::RendererMain(content::MainFunctionParams const&) + 1611
31  Electron Framework                  0x00000001095d3c27 content::ContentMainRunnerImpl::Run(bool) + 503
32  Electron Framework                  0x00000001095d2b32 content::RunContentProcess(content::ContentMainParams const&, content::ContentMainRunner*) + 2658
33  Electron Framework                  0x00000001095d2c1c content::ContentMain(content::ContentMainParams const&) + 44
34  Electron Framework                  0x0000000107c52297 ElectronMain + 135
35  Electron Helper (Renderer)          0x00000001040ad1b7 main + 439
36  libdyld.dylib                       0x00007fff694e6cc9 start + 1

I believe this is due to the hack in UvRunOnce being insufficient to swap between kExplicit and kScoped microtask policies when a new node environment is created during execution of UvRunOnce. i.e. this:

electron/shell/common/node_bindings.cc

Lines 580 to 586 in fa70301

	// Node.js expects `kExplicit` microtasks policy and will run microtasks
	// checkpoints after every call into JavaScript. Since we use a different
	// policy in the renderer - switch to `kExplicit` and then drop back to the
	// previous policy value.
	auto old_policy = env->isolate()->GetMicrotasksPolicy();
	DCHECK_EQ(v8::MicrotasksScope::GetCurrentDepth(env->isolate()), 0);
	env->isolate()->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit);

sets the policy to kExplicit, but then CreateEnvironment is called during the tick, and subsequently this:

electron/shell/common/node_bindings.cc

Line 518 in fa70301

node::SetIsolateUpForNode(context->GetIsolate(), is);

is called, which sets the policy back to kScoped, due to:

electron/shell/common/node_bindings.cc

Lines 490 to 492 in fa70301

	// Match Blink's behavior by allowing microtasks invocation to be controlled
	// by MicrotasksScope objects.
	is.policy = v8::MicrotasksPolicy::kScoped;

Since the policy is now kScoped, the DCHECK in question triggers the next time a JS function is called, which is in the CheckImmediate code in Node.

cc @codebytere

[codebytere]

cc @indutny - you may also have thoughts here! This was added in #28957

[indutny]

Thanks for a detailed report @nornagon , and sorry my changes caused it. I'm trying to find a solution that would account for code branches. Will get back to you once I have something...

[indutny]

The simplest solution might be to restore the policy at the end of the CreateEnvironment to whatever it was before instead of overwriting the current policy. I'm trying to make sure that no browser code would be called that creates MicrotasksScope instances on stack, and this takes time to verify...

[indutny-signal]

Here's the stack trace for the CreateEnvironment call that causes it:

* thread #1, name = 'CrRendererMain', queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x0000000111588a08 Electron Framework`::CreateEnvironment() at node_bindings.cc:393:8 [opt]
    frame #1: 0x00000001115b0783 Electron Framework`::DidCreateScriptContext() at electron_renderer_client.cc:111:23 [opt]
    frame #2: 0x00000001115af032 Electron Framework`::DidInstallConditionalFeatures() at electron_render_frame_observer.cc:71:23 [opt]
    frame #3: 0x000000011ae21bc5 Electron Framework`::DidInstallConditionalFeatures() at render_frame_impl.cc:4265:14 [opt]
    frame #4: 0x0000000119363495 Electron Framework`::Initialize() at local_window_proxy.cc:190:25 [opt]
    frame #5: 0x000000011864a6ca Electron Framework`::GetWindowProxy() [inlined] GetWindowProxy at window_proxy_manager.h:45:19 [opt]
    frame #6: 0x000000011864a6ba Electron Framework`::GetWindowProxy() at frame.cc:257 [opt]
    frame #7: 0x000000011863b30d Electron Framework`::Wrap() at dom_window.cc:68:17 [opt]
    frame #8: 0x0000000119f0a04f Electron Framework`::OpenOperationCallback() [inlined] SetWrapper<v8::FunctionCallbackInfo<v8::Value> > at v8_set_return_value.h:69:21 [opt]
    frame #9: 0x0000000119f0a03a Electron Framework`::OpenOperationCallback() [inlined] V8SetReturnValue<v8::FunctionCallbackInfo<v8::Value> > at v8_set_return_value.h:358 [opt]
    frame #10: 0x0000000119f0a02e Electron Framework`::OpenOperationCallback() at v8_window.cc:19161 [opt]
    frame #11: 0x0000000112e920f9 Electron Framework`::Call() at api-arguments-inl.h:156:3 [opt]
    frame #12: 0x0000000112e8fe9d Electron Framework`::HandleApiCallHelper<false>() at builtins-api.cc:112:36 [opt]
    frame #13: 0x0000000112e8d863 Electron Framework`::Builtin_Impl_HandleApiCall() at builtins-api.cc:142:5 [opt]
    frame #14: 0x0000000112e8d3ad Electron Framework`::Builtin_HandleApiCall() at builtins-api.cc:130:1 [opt]
    frame #15: 0x000000390008ce78
    frame #16: 0x000000390000e101
    frame #17: 0x000000390000e101
    frame #18: 0x000000390000bb5b
    frame #19: 0x000000390000b8e7
    frame #20: 0x0000000113029fdc Electron Framework`::Invoke() [inlined] Call at simulator.h:144:12 [opt]
    frame #21: 0x0000000113029fce Electron Framework`::Invoke() at execution.cc:376 [opt]
    frame #22: 0x00000001130289db Electron Framework`::Call() at execution.cc:470:10 [opt]
    frame #23: 0x0000000112ddee96 Electron Framework`::Call() at api.cc:5048:7 [opt]
    frame #24: 0x000000011b0bd622 Electron Framework`::InternalMakeCallback() at callback.cc:195:21 [opt]
    frame #25: 0x000000011b0bd8eb Electron Framework`::MakeCallback() at callback.cc:264:7 [opt]
    frame #26: 0x000000011b106a25 Electron Framework`::CheckImmediate() at env.cc:870:5 [opt]
    frame #27: 0x00000001113c696d Electron Framework`uv__run_check at loop-watcher.c:67:1 [opt]
    frame #28: 0x00000001113c08a1 Electron Framework`uv_run at core.c:394:5 [opt]
    frame #29: 0x0000000111589edc Electron Framework`::UvRunOnce() at node_bindings.cc:581:11 [opt]

[indutny]

Here goes the fix: #29531 .