Debug build assertion failure in V8 for sequential/test-inspector-contexts

[rvagg]

(as per #17016 & #17017)

Testing out adding Debug builds to CI and I have some failures that need looking at that'll need someone cleverer than me.

Current master, fails via sequential/test-inspector-contexts, on both Linux and macOS:

$ ./out/Debug/node --expose-gc test/sequential/test-inspector-contexts
Testing context created/destroyed notifications
Testing breakpoint is hit in a new context


#
# Fatal error in ../deps/v8/src/execution.cc, line 107
# Check failed: AllowJavascriptExecution::IsAllowed(isolate).
#
Illegal instruction: 4

/cc @nodejs/v8

[hashseed]

The issue here is that v8::ScriptCompiler::CompileUnboundInternal creates a DisallowJavaScriptExecution scope with ENTER_V8_NO_SCRIPT, but node::AsyncWrap::MakeCallback performs a callback, in the same Isolate.

Compilation should not secretly recurse and interleave with execution. Would it be possible to schedule this callback to later?

[addaleax]

@eugeneo ^^^

[addaleax]

@hashseed I just remembered that I already fixed this upstream in v8/v8@6751db2 – Are you surprised to hear you reviewed that code? 😄

[hashseed]

Ugh. I had the feeling that I've seen that code recently. Good that it's already fixed! Bad that we don't have a good way to detect if something is already fixed. I remember spending time on fixing node issues too where there is already a fix upstream.

[addaleax]

I guess the best way to do that would be testing things out on one of the canary repos? I’ve kind of lost track of how v8/node and our own canary repo are doing (I thought it was called node/node-canary but I can’t seem to find it right now? @targos ?)

[targos]

Our canary repo: https://github.com/nodejs/node-v8
Nightly builds from it: https://nodejs.org/download/v8-canary/

[hashseed]

V8's vee-eight-lkgr branch only syncs with upstream node once every 2-3 months so probably not the best for this purpose.

[addaleax]

This has been fixed by 04aab13 by now :)