Segmentation fault in worker test

@addaleax

On canary-base, there is currently one test failure:

out/Release/node test/parallel/test-worker-sharedarraybuffer-from-worker-thread.js        
[1]    11828 segmentation fault (core dumped)  out/Release/node

Backtrace:

#0  0x0000000000000020 in ?? ()
#1  0x0000000000e7a9f9 in v8::internal::BackingStore::~BackingStore() ()
#2  0x0000000000ba6dab in std::_Sp_counted_deleter<v8::internal::BackingStore*, std::default_delete<v8::internal::BackingStore>, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() ()
#3  0x0000000000d11a51 in v8::internal::ArrayBufferTracker::FreeAll(v8::internal::Page*) ()
#4  0x0000000000d11b90 in v8::internal::ArrayBufferTracker::TearDown(v8::internal::Heap*) ()
#5  0x0000000000d60b8c in v8::internal::Heap::TearDown() ()
#6  0x0000000000cfb6e3 in v8::internal::Isolate::Deinit() ()
#7  0x0000000000cfd57e in v8::internal::Isolate::Delete(v8::internal::Isolate*)
    ()
#8  0x0000000000a5c209 in node::NodeMainInstance::~NodeMainInstance() ()
#9  0x00000000009e738b in node::Start(int, char**) ()
#10 0x00007f81dba05f33 in __libc_start_main () from /lib64/libc.so.6
#11 0x0000000000951abe in _start ()

/cc @addaleax

Backtrace with debug build:

#0  0x000000000156b765 in v8::internal::BackingStore::~BackingStore (this=0x7f523813abf0, __in_chrg=<optimized out>) at ../../deps/v8/src/objects/backing-store.cc:157
#1  0x00000000010d8102 in std::default_delete<v8::internal::BackingStore>::operator() (this=<optimized out>, __ptr=0x7f523813abf0) at /usr/include/c++/9/bits/unique_ptr.h:75
#2  std::default_delete<v8::internal::BackingStore>::operator() (this=<optimized out>, __ptr=0x7f523813abf0) at /usr/include/c++/9/bits/unique_ptr.h:75
#3  std::_Sp_counted_deleter<v8::internal::BackingStore*, std::default_delete<v8::internal::BackingStore>, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (
    this=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:471
#4  0x0000000001381a55 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7f5238121cb0) at /usr/include/c++/9/ext/atomicity.h:69
#5  std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7f5238121cb0) at /usr/include/c++/9/bits/shared_ptr_base.h:148
#6  std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:730
#7  std::__shared_ptr<v8::internal::BackingStore, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<optimized out>, __in_chrg=<optimized out>)
    at /usr/include/c++/9/bits/shared_ptr_base.h:1169
#8  std::shared_ptr<v8::internal::BackingStore>::~shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr.h:103
#9  std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >::~pair (this=<optimized out>, __in_chrg=<optimized out>)
    at /usr/include/c++/9/bits/stl_pair.h:208
#10 __gnu_cxx::new_allocator<std::__detail::_Hash_node<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, true> >::destroy<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > > (this=<optimized out>, __p=<optimized out>) at /usr/include/c++/9/ext/new_allocator.h:153
#11 std::allocator_traits<std::allocator<std::__detail::_Hash_node<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, true> > >::destroy<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > > (__a=..., __p=<optimized out>) at /usr/include/c++/9/bits/alloc_traits.h:497
#12 std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, true> > >::_M_deallocate_node (this=<optimized out>, __n=<optimized out>) at /usr/include/c++/9/bits/hashtable_policy.h:2102
#13 std::_Hashtable<v8::internal::JSArrayBuffer, std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, std::allocator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > >, std::__detail::_Select1st, std::equal_to<v8::internal::JSArrayBuffer>, v8::internal::LocalArrayBufferTracker::Hasher, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_M_erase (__n=<optimized out>, 
    __prev_n=<optimized out>, __bkt=<optimized out>, this=<optimized out>) at /usr/include/c++/9/bits/hashtable.h:1886
#14 std::_Hashtable<v8::internal::JSArrayBuffer, std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, std::allocator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > >, std::__detail::_Select1st, std::equal_to<v8::internal::JSArrayBuffer>, v8::internal::LocalArrayBufferTracker::Hasher, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::erase (__it=..., 
    this=<optimized out>) at /usr/include/c++/9/bits/hashtable.h:1861
#15 std::_Hashtable<v8::internal::JSArrayBuffer, std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, std::allocator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > >, std::__detail::_Select1st, std::equal_to<v8::internal::JSArrayBuffer>, v8::internal::LocalArrayBufferTracker::Hasher, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::erase (__it=..., 
    this=<optimized out>) at /usr/include/c++/9/bits/hashtable.h:768
#16 std::unordered_map<v8::internal::JSArrayBuffer, std::shared_ptr<v8::internal::BackingStore>, v8::internal::LocalArrayBufferTracker::Hasher, std::equal_to<v8::internal::JSArrayBuffer>, std::allocator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > > >::erase (__position=..., this=<optimized out>)
    at /usr/include/c++/9/bits/unordered_map.h:798
#17 v8::internal::LocalArrayBufferTracker::Free<v8::internal::ArrayBufferTracker::FreeAll(v8::internal::Page*)::<lambda(v8::internal::JSArrayBuffer)> > (this=<optimized out>, 
    should_free=...) at ../../deps/v8/src/heap/array-buffer-tracker-inl.h:106
#18 v8::internal::ArrayBufferTracker::FreeAll (page=page@entry=0x3a1677780000) at ../../deps/v8/src/heap/array-buffer-tracker.cc:106
#19 0x0000000001381d38 in v8::internal::ArrayBufferTracker::TearDown (heap=heap@entry=0x40f27c0) at ../../deps/v8/src/heap/array-buffer-tracker.cc:148
#20 0x00000000013fb656 in v8::internal::Heap::TearDown (this=0x40f27c0) at ../../deps/v8/src/heap/heap.cc:5286
#21 0x000000000135c857 in v8::internal::Isolate::Deinit (this=0x40e94a0) at ../../deps/v8/src/execution/isolate.cc:3029
#22 0x000000000135eb55 in v8::internal::Isolate::Delete (isolate=0x40e94a0) at ../../deps/v8/src/execution/isolate.cc:2850
#23 0x0000000000f30e67 in node::NodeMainInstance::~NodeMainInstance (this=0x7fffbb09f010, __in_chrg=<optimized out>) at ../../src/node_main_instance.cc:91
#24 0x0000000000e8e56e in node::Start (argc=2, argv=0x7fffbb09f298) at ../../src/node.cc:1077
#25 0x00000000024dbd02 in main (argc=2, argv=0x7fffbb09f298) at ../../src/node_main.cc:126

(gdb) info registers
rax            0x0                 0
rbx            0x7f523813abf0      139991104859120
rcx            0x0                 0
rdx            0x2                 2
rsi            0x0                 0
rdi            0x423a820           69445664
rbp            0x7f523813abf8      0x7f523813abf8
rsp            0x7fffbb09ee80      0x7fffbb09ee80
r8             0x3c296e8           63084264
r9             0x0                 0
r10            0x78                120
r11            0x8                 8
r12            0x423a820           69445664
r13            0x0                 0
r14            0x4                 4
r15            0x3a1677780000      63868168044544
rip            0x156b765           0x156b765 <v8::internal::BackingStore::~BackingStore()+325>
eflags         0x10246             [ PF ZF IF RF ]
cs             0x33                51
ss             0x2b                43
ds             0x0                 0
es             0x0                 0
fs             0x0                 0
gs             0x0                 0

The line that fails: https://github.com/nodejs/node/blob/ebfb709b5f095c43f528affaaafe027efe6c223d/deps/v8/src/objects/backing-store.cc#L157

Thanks for the ping, I’ll look into it

@hashseed

I think this is v8/v8@b6b7de0 … since there are no accompanying API changes in V8, I think code that worked before should also work after, so this might be a bug in V8? I’ll still try to figure out what’s happening here exactly, but I think there might be a chance that V8 is trying to free an externalized SharedArrayBuffer instance on Isolate teardown.

Is Node.js still part of the V8 CI? https://github.com/v8/node/ doesn’t seem up to date at all… /cc @hashseed @titzer

Here is the node V8 CI run for that commit: https://ci.chromium.org/p/node-ci/builders/try/node_ci_linux64_rel/b8906314882042544400
It is green but I don't see this test in the output.

Of course, you added the test less than a month ago, so without an up to date v8/node repo it won't be executed by the V8 CI

I think there might be a chance that V8 is trying to free an externalized SharedArrayBuffer instance on Isolate teardown.

Yes, this is it. It might be possible to work around this in Node.js, but the new semantics aren’t really clear to me at a glance.

Hm yeah, I’m a bit stumped. The V8 change requires the ArrayBuffer::Allocator behind a SharedArrayBuffer to have a longer lifetime than previously, and while the change is one in the right direction, the different timing means that V8 tries to access it after we destroyed it.

@targos

So … what we could do, I think, is to store all ArrayBuffer::Allocator instances referenced by a Node.js Environment in a separate array that outlives the Environment, and then release those after the Isolate has been torn down – this isn’t a good solution, though, and one that requires a breaking change to our embedder API, at least temporarily (if we want to avoid memory leaks).

@targos What Node.js release lines would the V8 change affect? Is it something that would align with a semver-major release on our side?

This change won't land on time for Node.js 13.0.0.
There are at least two options:

We make the necessary breaking API changes before 13.0.0, allowing us to upgrade V8 in a semver-minor when the version containing this change is released.
We wait for the integration of this change in Node.js to adapt our API, locking us to the previous V8 version until Node.js 14.0.0 is released.

worker: keep allocators for transferred SAB instances alive longer

@targos

@targos Sometimes it’s good to get a bit of distance and think about it again :) Found a way to address this without API changes: nodejs/node#29637

worker: keep allocators for transferred SAB instances alive longer

test: expand Worker test for non-shared ArrayBuffer

deps: V8: cherry-pick 6b0a953

test: expand Worker test for non-shared ArrayBuffer

deps: V8: cherry-pick 6b0a953

test: expand Worker test for non-shared ArrayBuffer

deps: V8: cherry-pick 6b0a953

Is this still an issue? I ask, because I've updated to backing store in Node 14 to solve the deprecation warnings; everything's kind of working, but the backing store is created... in not the main thread... and is being released in not the main thread; and is crashing in the deconstructor method of BackingStore...

Segmentation fault still happens when running node 12.18.4 with multi-worker-threads

Segmentation fault still happens when running node 12.18.4 with multi-worker-threads

I don't believe this was fixed in 12.x. Fixed in 14.0.0 though.

No, in 14, windows debug addons crash with release Node... have to build a debug node to use debug plugins, otherwise the arraybuffer delete uses the wrong runtime (uses the delete linked with the addon, and not the delete linked with node that created the object in the first place)
Edit:err, sorry, I guess 'this' doesn't fail... just backing stores.

worker: keep allocators for transferred SAB instances alive longer

Navigation Menu

Repository navigation

Segmentation fault in worker test #115

8 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Segmentation fault in worker test #115

Description

Activity

targos commented on Sep 11, 2019

targos commented on Sep 11, 2019

addaleax commented on Sep 11, 2019

addaleax commented on Sep 12, 2019

targos commented on Sep 12, 2019

targos commented on Sep 12, 2019

addaleax commented on Sep 12, 2019

addaleax commented on Sep 13, 2019

addaleax commented on Sep 13, 2019

targos commented on Sep 13, 2019

addaleax commented on Sep 20, 2019

8 remaining items

d3x0r commented on Jun 8, 2020

JingkunHu commented on Oct 2, 2020

Trott commented on Oct 3, 2020

d3x0r commented on Oct 4, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions