Repository navigation
Failed cctest EnvironmentTest.MultipleEnvironmentsPerIsolate in debug build #26736
Description
[ RUN ] EnvironmentTest.MultipleEnvironmentsPerIsolate
#
# Fatal error in ../deps/v8/src/heap/heap.cc, line 4778
# Debug check failed: gc_prologue_callbacks_.end() == std::find(gc_prologue_callbacks_.begin(), gc_prologue_callbacks_.end(), GCCallbackTuple(callback, gc_type, data)).
#
#
#
#FailureMessage Object: 0x7ffeefbfdd90make[1]: *** [cctest] Illegal instruction: 4
Stack trace
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
* frame #0: 0x000000010197b512 cctest`v8::base::OS::Abort() at platform-posix.cc:400 [opt]
frame #1: 0x0000000101973b75 cctest`v8::base::(anonymous namespace)::DefaultDcheckHandler(file=<unavailable>, line=<unavailable>, message=<unavailable>) at logging.cc:56 [opt]
frame #2: 0x00000001006f70a3 cctest`v8::internal::Heap::AddGCPrologueCallback(this=0x00000000000012aa, callback=(0x00007ffeefbfe080), gc_type=<unavailable>, data=0x000000010215fb1d)(v8::Isolate*, v8::GCType, v8::GCCallbackFlags, void*), v8::GCType, void*) at heap.cc:4776 [opt]
frame #3: 0x00000001015833f9 cctest`node::InitDTrace(env=0x000000010481ec00) at node_dtrace.cc:275
frame #4: 0x0000000101298edd cctest`node::RunBootstrapping(env=0x000000010481ec00) at node.cc:258
frame #5: 0x000000010114c4e7 cctest`EnvironmentTestFixture::Env::Env(this=0x00007ffeefbfedf8, handle_scope=0x00007ffeefbfee38, argv=0x00007ffeefbfee28) at node_test_fixture.h:135
frame #6: 0x000000010114abf5 cctest`EnvironmentTestFixture::Env::Env(this=0x00007ffeefbfedf8, handle_scope=0x00007ffeefbfee38, argv=0x00007ffeefbfee28) at node_test_fixture.h:119
frame #7: 0x00000001011547dc cctest`EnvironmentTest_MultipleEnvironmentsPerIsolate_Test::TestBody(this=0x00000001044002a0) at test_environment.cc:61
frame #8: 0x00000001016d9a4e cctest`void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(object=0x00000001044002a0, method=21 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00, location="the test body")(), char const*) at gtest.cc:2364
frame #9: 0x00000001016afc42 cctest`void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(object=0x00000001044002a0, method=21 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00, location="the test body")(), char const*) at gtest.cc:2417
frame #10: 0x00000001016afb16 cctest`testing::Test::Run(this=0x00000001044002a0) at gtest.cc:2436
frame #11: 0x00000001016b0b8d cctest`testing::TestInfo::Run(this=0x0000000104206170) at gtest.cc:2612
frame #12: 0x00000001016b1e9c cctest`testing::TestCase::Run(this=0x0000000104205e00) at gtest.cc:2730
frame #13: 0x00000001016c0ddc cctest`testing::internal::UnitTestImpl::RunAllTests(this=0x0000000104203f00) at gtest.cc:4726
frame #14: 0x00000001016dd55e cctest`bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x0000000104203f00, method=50 0a 6c 01 01 00 00 00 00 00 00 00 00 00 00 00, location="auxiliary test code (environments or event listeners)")(), char const*) at gtest.cc:2364
frame #15: 0x00000001016c09d2 cctest`bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x0000000104203f00, method=50 0a 6c 01 01 00 00 00 00 00 00 00 00 00 00 00, location="auxiliary test code (environments or event listeners)")(), char const*) at gtest.cc:2417
frame #16: 0x00000001016c08c9 cctest`testing::UnitTest::Run(this=0x0000000102b7bca0) at gtest.cc:4341
frame #17: 0x00000001016e1011 cctest`RUN_ALL_TESTS() at gtest.h:2326
frame #18: 0x00000001016e0feb cctest`main(argc=1, argv=0x00007ffeefbff688) at gtest_main.cc:36
frame #19: 0x0000000100001034 cctest`start + 52
Activity
joyeecheung commentedon Mar 18, 2019
The issue is when we have multiple environments accessing the same isolate, we install the same dtrace GC callbacks twice which triggers the DCHECK. I believe dtrace and ETW tracing is, similar to trace events in core, essentially global, and the GC callbacks don't seem to actually rely on the isolate, they just log the GC type and flags. So the fix should be either
NewIsolateI think 1 probably makes more sense
[-]Flaky cctest EnvironmentTest.MultipleEnvironmentsPerIsolate in debug build[/-][+]Failed cctest EnvironmentTest.MultipleEnvironmentsPerIsolate in debug build[/+]addaleax commentedon Mar 18, 2019
Yeah, I think either variant makes sense – add this to
SetIsolateUpForNode(), or track this via a field inIsolateData.As another solution, the easiest thing would probably be to use the variant of
AddGCPrologueCallback()that takes avoid*argument and store theEnvironment*in it? (And then also, ideally, use that to remove the hooks as required, as in #25647)? (The downside of that would be calling the handler multiple times.)joyeecheung commentedon Mar 18, 2019
Maybe we should just call
InitDtraceinStartNodeWithIsolate? Does it make sense to enable it for embedders who do not usenode::Startor for cctest any way?addaleax commentedon Mar 18, 2019
I don’t think it makes sense for cctest, but for embedders I would assume that the answer is “yes”. /cc @cjihrig
joyeecheung commentedon Mar 18, 2019
I tried with these two, but it seems too early call the current
InitDtraceduringSetIsolateUpForNode, because we can't know if it's on the main thread for certain by then.We could probably also split the GC callback part out of
InitDtrace, call it inSetIsolateUpForNode, and in the callback, do whatever state management necessary withEnvironment::GetCurrent. But that just fixes the test, the core issue is still whether the setup, includinginit_etw(), is necessary at all for uses outside of our binary. Since the tracing is global, initializing them multiple times for each isolate set up by Node may lead to unnecessary traces being written to the system.cjihrig commentedon Mar 18, 2019
That sounds fine to me.
joyeecheung commentedon Mar 18, 2019
You mean the embedders may need dtrace and ETW, or they do not?
Also cc @nodejs/embedders
cjihrig commentedon Mar 18, 2019
Sorry, I meant I think it makes sense to enable them for embedders even if they do not use
node::Start(I think that was the original question).joyeecheung commentedon Mar 18, 2019
On a side note, it looks like we can't use
Environment::GetCurrentin the dtrace callbacks because when the emebdder data slot is not set in debug mode, it triggers the fatal error handler, which er..triggersEnvironment::GetCurrentagain, then it starts infinite recursion (probably a separate issue for our fatal error handler, we could just pass the Environment as avoid *to the callback setter instead).src: fix DTrace GC callbacks DCHECKs and add cleanup