Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PerformanceObserver 'gc' crash with multiple observers #39548

Closed
rochdev opened this issue Jul 27, 2021 · 3 comments
Closed

PerformanceObserver 'gc' crash with multiple observers #39548

rochdev opened this issue Jul 27, 2021 · 3 comments
Labels
perf_hooks Issues and PRs related to the implementation of the Performance Timing API.

Comments

@rochdev
Copy link
Contributor

rochdev commented Jul 27, 2021

Version

16.5.0

Platform

Darwin COMP-C02DC1F0ML87 19.6.0 Darwin Kernel Version 19.6.0: Tue Jun 22 19:49:55 PDT 2021; root:xnu-6153.141.35~1/RELEASE_X86_64 x86_64

Subsystem

perf_hooks

What steps will reproduce the bug?

const { PerformanceObserver } = require('perf_hooks')

const gcObserver = new PerformanceObserver(list => {})

gcObserver.observe({ entryTypes: ['gc'] })

gcObserver.disconnect()

const gcObserver2 = new PerformanceObserver(list => {})

gcObserver2.observe({ entryTypes: ['gc'] })

gcObserver2.disconnect()

How often does it reproduce? Is there a required condition?

Every time when multiple observers are used and disconnected. In the reproduction example this happens synchronously but in our actual code the observer is disconnected later on asynchronously and then a new observer is created, which happens every 10 seconds.

What is the expected behavior?

Multiple observers for the same type should work.

What do you see instead?

#
# Fatal error in , line 0
# unreachable code
#
#
#
#FailureMessage Object: 0x7ffeef391260
 1: 0x1009adb82 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() [/Users/roch.devost/.nvm/versions/node/v16.1.0/bin/node]
 2: 0x101987cc2 V8_Fatal(char const*, ...) [/Users/roch.devost/.nvm/versions/node/v16.1.0/bin/node]
 3: 0x100ca17c9 v8::internal::Heap::RemoveGCPrologueCallback(void (*)(v8::Isolate*, v8::GCType, v8::GCCallbackFlags, void*), void*) [/Users/roch.devost/.nvm/versions/node/v16.1.0/bin/node]
 4: 0x1009a926f node::performance::RemoveGarbageCollectionTracking(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/roch.devost/.nvm/versions/node/v16.1.0/bin/node]
 5: 0x100b30275 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/Users/roch.devost/.nvm/versions/node/v16.1.0/bin/node]
 6: 0x100b2f848 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/roch.devost/.nvm/versions/node/v16.1.0/bin/node]
 7: 0x100b2edff v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/Users/roch.devost/.nvm/versions/node/v16.1.0/bin/node]
 8: 0x1013b87b9 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/Users/roch.devost/.nvm/versions/node/v16.1.0/bin/node]
zsh: illegal hardware instruction

Additional information

The crash happens on both macOS and Linux, and on every version of Node since 16.1.0. From what I can tell, it was caused by #38414 since there is no crash in previous Node versions (tested 12.0.0 - 16.0.0).

@legendecas legendecas added the perf_hooks Issues and PRs related to the implementation of the Performance Timing API. label Jul 27, 2021
@rochdev
Copy link
Contributor Author

rochdev commented Jul 27, 2021

Interestingly, it seems to only happen if all observers are disconnected. If multiple observers are created and disconnected while at least one is still observing, then there is no crash.

@jasnell
Copy link
Member

jasnell commented Jul 27, 2021

Investigating!

jasnell added a commit to jasnell/node that referenced this issue Jul 27, 2021
Signed-off-by: James M Snell <jasnell@gmail.com>
Fixes: nodejs#39548
jasnell added a commit to jasnell/node that referenced this issue Jul 27, 2021
Signed-off-by: James M Snell <jasnell@gmail.com>
Fixes: nodejs#39548
@jasnell
Copy link
Member

jasnell commented Jul 27, 2021

Fix in #39550

danielleadams pushed a commit that referenced this issue Aug 16, 2021
Signed-off-by: James M Snell <jasnell@gmail.com>
Fixes: #39548

PR-URL: #39550
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf_hooks Issues and PRs related to the implementation of the Performance Timing API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants