Repository navigation
Failed cctest EnvironmentTest.MultipleEnvironmentsPerIsolate in debug build #26736
Description
Activity
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
- Moving this to
NewIsolate - Making the subsequent calla a noop
I think 1 probably makes more sense
- Moving this to
- changed the title
[-]Flaky cctest EnvironmentTest.MultipleEnvironmentsPerIsolate in debug build[/-][+]Failed cctest EnvironmentTest.MultipleEnvironmentsPerIsolate in debug build[/+]on 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.)Maybe we should just call
InitDtraceinStartNodeWithIsolate? Does it make sense to enable it for embedders who do not usenode::Startor for cctest any way?add this to SetIsolateUpForNode(), or track this via a field in IsolateData.
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.As another solution, the easiest thing would probably be to use the variant of AddGCPrologueCallback() that takes a void* argument and store the Environment* in it?
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.That sounds fine to me.
You mean the embedders may need dtrace and ETW, or they do not?
Also cc @nodejs/embedders
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).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).- added a commit that references this issue
on Jul 27, 2026
Stack trace