node crashes when v8.getHeapSnapshot is called in succession. #35559

PoojaDurgad · 2020-10-08T16:24:22Z

Version: v14.13.1
Platform: linux
Subsystem: v8

What steps will reproduce the bug?

// given code
const v8 = require('v8');
v8.getHeapSnapshot()
v8.getHeapSnapshot()

What is the expected behavior?

no crash, the API returns streams

Additional information

I can easily recreate it, so any more info can be collected if needed.

joyeecheung · 2020-10-08T16:36:42Z

Can you post the stack trace here?

PoojaDurgad · 2020-10-09T06:24:14Z

@joyeecheung - Thanks for the reply and here is the stack trace of the code.

#1  0x0000000000fdb7b5 in v8::internal::wasm::WasmDecoder<(v8::internal::wasm::Decoder::ValidateFlag)1>::OpcodeLength(v8::internal::wasm::Decoder*, unsigned char const*) [clone .constprop.0] ()
#2  0x000000000102d046 in v8::internal::wasm::DecodeLocalNames(v8::internal::Vector<unsigned char const>) ()
#3  0x0000000000fdd1d9 in v8::internal::wasm::WasmCompilationUnit::CompileWasmFunction(v8::internal::Isolate*, v8::internal::wasm::NativeModule*, v8::internal::wasm::WasmFeatures*, v8::internal::wasm::WasmFunction const*, v8::internal::wasm::ExecutionTier) ()
#4  0x0000000000fde268 in v8::internal::wasm::Decoder::errorf(unsigned int, char const*, ...) [clone .constprop.0] ()
#5  0x0000000000fce530 in v8::internal::wasm::WasmFullDecoder<(v8::internal::wasm::Decoder::ValidateFlag)1, v8::internal::wasm::EmptyInterface>::DecodeNumericOpcode(v8::internal::wasm::WasmOpcode) ()
#6  0x00000000009b4ea6 in v8::internal::(anonymous namespace)::SetLengthProperty(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSReceiver>, double) ()
#7  0x0000000000be2f2b in v8::internal::Runtime_HasElementWithInterceptor(int, unsigned long*, v8::internal::Isolate*) ()
#8  0x0000000000be44d6 in v8::internal::IC::UpdatePolymorphicIC(v8::internal::Handle<v8::internal::Name>, v8::internal::MaybeObjectHandle const&) ()
#9  0x0000000000be4b56 in v8::internal::LoadIC::UpdateCaches(v8::internal::LookupIterator*) ()
#10 0x00000000013feab9 in BLAKE2b_Final ()
#11 0x0000000001397d62 in SSL_CTX_use_serverinfo_ex ()
#12 0x0000000001397d62 in SSL_CTX_use_serverinfo_ex ()
#13 0x0000000001397d62 in SSL_CTX_use_serverinfo_ex ()
#14 0x0000000001397d62 in SSL_CTX_use_serverinfo_ex ()
#15 0x0000000001397d62 in SSL_CTX_use_serverinfo_ex ()
#16 0x0000000001397d62 in SSL_CTX_use_serverinfo_ex ()
#17 0x0000000001397d62 in SSL_CTX_use_serverinfo_ex ()
#18 0x0000000001397d62 in SSL_CTX_use_serverinfo_ex ()
#19 0x0000000001395a7a in ssl_set_cert ()
#20 0x0000000001395858 in ssl_do_config ()
#21 0x0000000000cc0501 in v8::internal::(anonymous namespace)::ElementsAccessorBase<v8::internal::(anonymous namespace)::TypedElementsAccessor<(v8::internal::ElementsKind)20, short>, v8::internal::(anonymous namespace)::ElementsKindTraits<(v8::internal::ElementsKind)20> >::CopyTypedArrayElementsSlice(v8::internal::JSTypedArray, v8::internal::JSTypedArray, unsigned long, unsigned long) ()
#22 0x0000000000cc136f in v8::internal::(anonymous namespace)::ElementsAccessorBase<v8::internal::(anonymous namespace)::TypedElementsAccessor<(v8::internal::ElementsKind)19, unsigned short>, v8::internal::(anonymous namespace)::ElementsKindTraits<(v8::internal::ElementsKind)19> >::CopyTypedArrayElementsSlice(v8::internal::JSTypedArray, v8::internal::JSTypedArray, unsigned long, unsigned long) ()
#23 0x0000000000b89254 in v8::internal::YoungGenerationMarkingTask::MarkObject(v8::internal::Object) ()
#24 0x00000000009cd906 in v8::internal::Builtin_Impl_Stats_ConsoleContext(int, unsigned long*, v8::internal::Isolate*) ()
#25 0x00000000009cdb93 in v8::internal::Builtin_Impl_Stats_ConsoleTime(int, unsigned long*, v8::internal::Isolate*) ()
#26 0x00000000009cf1f0 in v8::internal::Builtin_ConsoleTimeStamp(int, unsigned long*, v8::internal::Isolate*) ()
#27 0x000000000096db6a in v8::debug::WasmScript::GetFunctionHash(int) ()
#28 0x0000000000000000 in ?? ()

addaleax · 2020-10-11T14:20:56Z

Just noticed that I ran into this independently of this issue – https://chromium-review.googlesource.com/c/v8/v8/+/2464823 fixes this.

RaisinTen · 2020-10-11T14:47:17Z

@addaleax so is this a problem with the v8 GC or node?

addaleax · 2020-10-11T14:49:32Z

@RaisinTen It’s a V8 bug, yes, not caused by the GC implementation but related to GC timing. We could work around it if necessary, but usually the V8 team responds very quickly during working hours (… faster than Node.js for sure 🙂 )

RaisinTen · 2020-10-11T15:13:45Z

@addaleax 🙂
I read it and this is what I understood:

When we query the first snapshot object, it gets created and returned
Then the second object is queried and the object creation is not complete yet
Due to the GC timing, the second object gets deleted along with the first one before we actually get to handle the second object
Now we are returned an address which we are not allowed to dereference, so we get a segfault

Did I get it right? I don't think I understand how we can work around this though.

addaleax · 2020-10-12T08:55:54Z

@RaisinTen Not quite… the process for creating a snapshot is:

V8 starts creating the snapshot, adding some strings to the heap profiler’s internal string table
V8 performs a GC, so that the heap snapshot does not contain objects that are not actually relevant to the snapshot
V8 continues creating the snapshot, adding more strings to the internal string table
Node.js wraps the snapshot (C++ object) in a JS object, using a weak reference – i.e. once the JS object is GC’ed, we delete the C++ snapshot object

Now… that last bit is a bit tricky: V8 implements “Delete the snapshot object” in 2 different ways:

If there is only one complete snapshot object, perform “delete all snapshot objects + reset metadata”, where “metadata” includes the internal string table
If there are more complete snapshot objects, delete only the particular snapshot object in question.

Part of the problem is the word “complete” here.

Now, when running the script from the issue here …

During creation of the second snapshot, in step 2, V8 performs a GC.
Since the first snapshot is weakly held, this deletes it.
The delete operation also deletes metadata, because at that point, the first snapshot is the only complete one
In step 3, the second snapshot tries to access the internal string table – the one that was deleted just before this
The process crashes or hangs 💥

Fixes: nodejs#35559

RaisinTen · 2020-10-12T16:07:45Z

@addaleax thank you for the awesome explanation! 🙂

Original commit message: [heap-profiler] Fix crash when a snapshot deleted while taking one Fix a crash/hang that occurred when deleting a snapshot during the GC that is part of taking another one. Specifically, when deleting the only other snapshot in such a situation, the `v8::HeapSnapshot::Delete()` method sees that there is only one (complete) snapshot at that point, and decides that it is okay to perform “delete all snapshots” instead of just deleting the requested one. That resets the internal string lookup table of the heap profiler, but the new snapshot that is currently in progress still holds references to the old string lookup table, leading to a use-after-free segfault or infinite loop. Fix this by guarding against resetting the string table while another heap snapshot is being taken, and add a test that would crash before this fix. This can be triggered in Node.js by repeatedly calling `v8.getHeapSnapshot()`, which provides heap snapshots as weakly held host objects. Change-Id: If9ac3728bf79114000982f1e7bb05e8034299e3c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2464823 Reviewed-by: Ulan Degenbaev <[email protected]> Commit-Queue: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#70445} Refs: v8/v8@3176bfd PR-URL: #35612 Refs: #35559 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>

Fixes: #35559 PR-URL: #35612 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>

Original commit message: [heap-profiler] Fix crash when a snapshot deleted while taking one Fix a crash/hang that occurred when deleting a snapshot during the GC that is part of taking another one. Specifically, when deleting the only other snapshot in such a situation, the `v8::HeapSnapshot::Delete()` method sees that there is only one (complete) snapshot at that point, and decides that it is okay to perform “delete all snapshots” instead of just deleting the requested one. That resets the internal string lookup table of the heap profiler, but the new snapshot that is currently in progress still holds references to the old string lookup table, leading to a use-after-free segfault or infinite loop. Fix this by guarding against resetting the string table while another heap snapshot is being taken, and add a test that would crash before this fix. This can be triggered in Node.js by repeatedly calling `v8.getHeapSnapshot()`, which provides heap snapshots as weakly held host objects. Change-Id: If9ac3728bf79114000982f1e7bb05e8034299e3c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2464823 Reviewed-by: Ulan Degenbaev <[email protected]> Commit-Queue: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#70445} Refs: v8/v8@3176bfd PR-URL: nodejs#35612 Refs: nodejs#35559 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>

Original commit message: [heap-profiler] Fix crash when a snapshot deleted while taking one Fix a crash/hang that occurred when deleting a snapshot during the GC that is part of taking another one. Specifically, when deleting the only other snapshot in such a situation, the `v8::HeapSnapshot::Delete()` method sees that there is only one (complete) snapshot at that point, and decides that it is okay to perform “delete all snapshots” instead of just deleting the requested one. That resets the internal string lookup table of the heap profiler, but the new snapshot that is currently in progress still holds references to the old string lookup table, leading to a use-after-free segfault or infinite loop. Fix this by guarding against resetting the string table while another heap snapshot is being taken, and add a test that would crash before this fix. This can be triggered in Node.js by repeatedly calling `v8.getHeapSnapshot()`, which provides heap snapshots as weakly held host objects. Change-Id: If9ac3728bf79114000982f1e7bb05e8034299e3c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2464823 Reviewed-by: Ulan Degenbaev <[email protected]> Commit-Queue: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#70445} Refs: v8/v8@3176bfd PR-URL: #35612 Refs: #35559 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>

Original commit message: [heap-profiler] Fix crash when a snapshot deleted while taking one Fix a crash/hang that occurred when deleting a snapshot during the GC that is part of taking another one. Specifically, when deleting the only other snapshot in such a situation, the `v8::HeapSnapshot::Delete()` method sees that there is only one (complete) snapshot at that point, and decides that it is okay to perform “delete all snapshots” instead of just deleting the requested one. That resets the internal string lookup table of the heap profiler, but the new snapshot that is currently in progress still holds references to the old string lookup table, leading to a use-after-free segfault or infinite loop. Fix this by guarding against resetting the string table while another heap snapshot is being taken, and add a test that would crash before this fix. This can be triggered in Node.js by repeatedly calling `v8.getHeapSnapshot()`, which provides heap snapshots as weakly held host objects. Change-Id: If9ac3728bf79114000982f1e7bb05e8034299e3c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2464823 Reviewed-by: Ulan Degenbaev <[email protected]> Commit-Queue: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#70445} Refs: v8/v8@3176bfd PR-URL: nodejs#35612 Refs: nodejs#35559 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>

Original commit message: [heap-profiler] Fix crash when a snapshot deleted while taking one Fix a crash/hang that occurred when deleting a snapshot during the GC that is part of taking another one. Specifically, when deleting the only other snapshot in such a situation, the `v8::HeapSnapshot::Delete()` method sees that there is only one (complete) snapshot at that point, and decides that it is okay to perform “delete all snapshots” instead of just deleting the requested one. That resets the internal string lookup table of the heap profiler, but the new snapshot that is currently in progress still holds references to the old string lookup table, leading to a use-after-free segfault or infinite loop. Fix this by guarding against resetting the string table while another heap snapshot is being taken, and add a test that would crash before this fix. This can be triggered in Node.js by repeatedly calling `v8.getHeapSnapshot()`, which provides heap snapshots as weakly held host objects. Change-Id: If9ac3728bf79114000982f1e7bb05e8034299e3c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2464823 Reviewed-by: Ulan Degenbaev <[email protected]> Commit-Queue: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#70445} Refs: v8/v8@3176bfd PR-URL: #35612 Refs: #35559 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>

Original commit message: [heap-profiler] Fix crash when a snapshot deleted while taking one Fix a crash/hang that occurred when deleting a snapshot during the GC that is part of taking another one. Specifically, when deleting the only other snapshot in such a situation, the `v8::HeapSnapshot::Delete()` method sees that there is only one (complete) snapshot at that point, and decides that it is okay to perform “delete all snapshots” instead of just deleting the requested one. That resets the internal string lookup table of the heap profiler, but the new snapshot that is currently in progress still holds references to the old string lookup table, leading to a use-after-free segfault or infinite loop. Fix this by guarding against resetting the string table while another heap snapshot is being taken, and add a test that would crash before this fix. This can be triggered in Node.js by repeatedly calling `v8.getHeapSnapshot()`, which provides heap snapshots as weakly held host objects. Change-Id: If9ac3728bf79114000982f1e7bb05e8034299e3c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2464823 Reviewed-by: Ulan Degenbaev <[email protected]> Commit-Queue: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#70445} Refs: v8/v8@3176bfd PR-URL: nodejs#35612 Refs: nodejs#35559 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>

Fixes: nodejs#35559 PR-URL: nodejs#35612 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>

OmgImAlexis · 2021-02-15T01:45:32Z

Has this landed in a version of node yet?

I'm trying to debug using chrome's "Allocation instrumentation on timeline" profile and it crashes with a segfault which looks similar to this. If it's not please let me know and I'll open my own issue. I commented here as it's the only thing I could find on google when searching for this.

/usr/local/bin/node/unraid-api/node_modules/segfault-handler/build/Release/segfault-handler.node(+0x3276)[0x149965609276]
/lib64/libpthread.so.0(+0x13690)[0x149966410690]
unraid-api(_ZN2v88internal14V8HeapExplorer24ExtractContextReferencesEPNS0_9HeapEntryENS0_7ContextE+0xd6)[0x104ba06]
unraid-api(_ZN2v88internal14V8HeapExplorer27IterateAndExtractReferencesEPNS0_21HeapSnapshotGeneratorE+0x385)[0x1050f75]
unraid-api(_ZN2v88internal21HeapSnapshotGenerator16GenerateSnapshotEv+0xd4)[0x1051c14]
unraid-api(_ZN2v88internal12HeapProfiler12TakeSnapshotEPNS_15ActivityControlEPNS_12HeapProfiler18ObjectNameResolverEb+0x6f)[0x1041bef]
unraid-api(_ZN12v8_inspector23V8HeapProfilerAgentImpl16takeHeapSnapshotEN8v8_crdtp4glue6detail10ValueMaybeIbEES5_+0xf5)[0x12efe05]
unraid-api(_ZN12v8_inspector23V8HeapProfilerAgentImpl23stopTrackingHeapObjectsEN8v8_crdtp4glue6detail10ValueMaybeIbEES5_+0x96)[0x12f10b6]
unraid-api(_ZN12v8_inspector8protocol12HeapProfiler20DomainDispatcherImpl23stopTrackingHeapObjectsERKN8v8_crdtp12DispatchableEPNS0_15DictionaryValueEPNS3_12ErrorSupportE+0x191)[0x13cfb71]
unraid-api[0x13ceede]
unraid-api(_ZN8v8_crdtp14UberDispatcher14DispatchResult3RunEv+0x1b)[0x132875b]
unraid-api(_ZN12v8_inspector22V8InspectorSessionImpl23dispatchProtocolMessageENS_10StringViewE+0x19f)[0x12f7f5f]
unraid-api[0xb1774a]
unraid-api[0xb374b9]
unraid-api[0xb3572e]
unraid-api[0xb35a81]
unraid-api[0xb35d8d]
unraid-api(_ZN4node11Environment21RunAndClearInterruptsEv+0x140)[0x9b2af0]
unraid-api[0x9b2b58]
unraid-api(_ZN2v88internal7Isolate27InvokeApiInterruptCallbacksEv+0x8b)[0xd0bdbb]
unraid-api(_ZN2v88internal10StackGuard16HandleInterruptsEv+0x1fd)[0xd2379d]
unraid-api(_ZN2v88internal18Runtime_StackGuardEiPmPNS0_7IsolateE+0x5a)[0x10cacfa]
unraid-api[0x1483939]
Segmentation fault

addaleax · 2021-02-15T10:04:31Z

@OmgImAlexis The stack trace looks very different so I’d open a new issue. (And yes, the fix for this has been released on the latest 15.x and 14.x branches, but it’s unrelated to the Chrome feature anyway.)

divasatanica · 2023-08-10T13:53:49Z

@addaleax Hi, I met the same issue as @OmgImAlexis , can you post the link of the new issue you mentioned above? thanks

joyeecheung added memory Issues and PRs related to the memory management or memory footprint. v8 module Issues and PRs related to the "v8" subsystem. labels Oct 8, 2020

gireeshpunathil mentioned this issue Oct 10, 2020

Segfault while running JS code #35581

Closed

RaisinTen mentioned this issue Oct 11, 2020

Debugging Node nodejs/help#3016

Closed

addaleax added a commit to addaleax/node that referenced this issue Oct 12, 2020

test: add regression test for v8.getHeapSnapshot() crash

4fbf127

Fixes: nodejs#35559

addaleax mentioned this issue Oct 12, 2020

deps: V8: cherry-pick 3176bfd447a9 #35612

Closed

3 tasks

addaleax closed this as completed in 6265951 Oct 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

node crashes when v8.getHeapSnapshot is called in succession. #35559

node crashes when v8.getHeapSnapshot is called in succession. #35559

PoojaDurgad commented Oct 8, 2020

joyeecheung commented Oct 8, 2020

PoojaDurgad commented Oct 9, 2020

addaleax commented Oct 11, 2020

RaisinTen commented Oct 11, 2020

addaleax commented Oct 11, 2020

RaisinTen commented Oct 11, 2020

addaleax commented Oct 12, 2020

RaisinTen commented Oct 12, 2020

OmgImAlexis commented Feb 15, 2021

addaleax commented Feb 15, 2021

divasatanica commented Aug 10, 2023

node crashes when v8.getHeapSnapshot is called in succession. #35559

node crashes when v8.getHeapSnapshot is called in succession. #35559

Comments

PoojaDurgad commented Oct 8, 2020

What steps will reproduce the bug?

What is the expected behavior?

Additional information

joyeecheung commented Oct 8, 2020

PoojaDurgad commented Oct 9, 2020

addaleax commented Oct 11, 2020

RaisinTen commented Oct 11, 2020

addaleax commented Oct 11, 2020

RaisinTen commented Oct 11, 2020

addaleax commented Oct 12, 2020

RaisinTen commented Oct 12, 2020

OmgImAlexis commented Feb 15, 2021

addaleax commented Feb 15, 2021

divasatanica commented Aug 10, 2023