Skip to content

exit race between main and worker threads #25007

Closed
Closed
exit race between main and worker threads#25007
Labels
c++Issues and PRs that require attention from people who are familiar with C++.confirmed-bugIssues and PRs for confirmed bugs.lib / srcIssues and PRs involving general changes in the lib/ or src/ directories.processIssues and PRs related to the process subsystem.

Description

@gireeshpunathil
gireeshpunathil
opened on Dec 13, 2018
  • Version: v11.0.0
  • Platform: all
  • Subsystem: worker, process, src

Sample test case to reproduce the issue:

'use strict'
const { Worker, isMainThread, parentPort } = require('worker_threads')

if (isMainThread) {
  const count = process.argv[2] / 1
  for(var i=0;i<count;i++)
    new Worker(__filename)
  process.exit(0)
} else {
  setInterval(() => {
    parentPort.postMessage('Hello, world!')
  }, 1)
}

The flakiness of the test is largely influenced by the thread scheduling order / number of CPUs / load on the system.

First reported in AIX and Linux through sequential/test-cli-syntax.js. The more you run, the more variety of scenarios you get: SIGSEGV, SIGABRT, SIGILL... depends on at what point the main and the worker threads are.

The root cause is that there is no specified order / identified ownership of C++ global objects that being destructed between threads.

Refs: #24403

Activity

  1. gireeshpunathil
    added
    confirmed-bugIssues and PRs for confirmed bugs.
    processIssues and PRs related to the process subsystem.
    lib / srcIssues and PRs involving general changes in the lib/ or src/ directories.
    workerIssues and PRs related to the worker_threads module and Worker API.
    on Dec 13, 2018

  2. gireeshpunathil commented on Dec 13, 2018

    @gireeshpunathil
    gireeshpunathil
    on Dec 13, 2018
    MemberAuthor

    /cc @nodejs/workers @nodejs/process

  3. gireeshpunathil commented on Dec 13, 2018

    @gireeshpunathil
    gireeshpunathil
    on Dec 13, 2018
    MemberAuthor

    on a side note: why would worker module be loaded even in the absence of --experimental-worker flag? I believe the answer is that all the internal modules are loaded at bootstrap, irrespective of their requirement at runtime.

    Can we change that? At least in this case, it will save a lot of resources (thread, memory) for use cases that do not require worker?

  4. Trott commented on Dec 13, 2018

    @Trott
    Trott
    on Dec 13, 2018
    Member

    @bengl was looking at this earlier but I don't know if he has anything to add (and if he did, he'd probably put it in the other issue). Pinging him here anyway just in case...

  5. addaleax commented on Dec 13, 2018

    @addaleax
    addaleax
    on Dec 13, 2018
    Member

    why would worker module be loaded even in the absence of --experimental-worker flag?

    As far as I can tell, it’s only the native binding that’s loaded unconditionally (to figure out whether we’re in a worker or not during bootstrap).

    I believe the answer is that all the internal modules are loaded at bootstrap, irrespective of their requirement at runtime.

    That’s not the case; test/parallel/test-bootstrap-modules.js tests this.

    At least in this case, it will save a lot of resources (thread, memory) for use cases that do not require worker?

    The memory overhead is probably not huge, and just loading the worker module does not spawn any threads on its own.

  6. gireeshpunathil commented on Dec 13, 2018

    @gireeshpunathil
    gireeshpunathil
    on Dec 13, 2018
    MemberAuthor
    $ gdb ./node_g
(gdb) b pthread_create
Breakpoint 1 at 0xd2fe40
(gdb) r
Starting program: ./node_g 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, 0x0000000000d2fe40 in pthread_create@plt ()
Missing separate debuginfos, use: debuginfo-install glibc-2.17-222.el7.x86_64 libgcc-4.8.5-28.el7.x86_64 libstdc++-4.8.5-28.el7.x86_64
(gdb) bt
#0  0x0000000000d2fe40 in pthread_create@plt ()
#1  0x0000000000fd8acf in uv_thread_create (tid=0x3396790, 
    entry=0xe9be78 <node::WorkerThreadsTaskRunner::DelayedTaskScheduler::Start()::{lambda(void*)#1}::_FUN(void*)>, arg=0x3395f60) at ../deps/uv/src/unix/thread.c:213
#2  0x0000000000e9bf1a in node::WorkerThreadsTaskRunner::DelayedTaskScheduler::Start (
    this=0x3395f60) at ../src/node_platform.cc:63
#3  0x0000000000e998e8 in node::WorkerThreadsTaskRunner::WorkerThreadsTaskRunner (
    this=0x3395bf0, thread_pool_size=4) at ../src/node_platform.cc:178
#4  0x0000000000ea795f in __gnu_cxx::new_allocator<node::WorkerThreadsTaskRunner>::construct<node::WorkerThreadsTaskRunner<int&> > (this=0x7fffffffdcfd, __p=0x3395bf0)
    at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/ext/new_allocator.h:120
#5  0x0000000000ea68c4 in std::allocator_traits<std::allocator<node::WorkerThreadsTaskRunner> >::_S_construct<node::WorkerThreadsTaskRunner<int&> >(std::allocator<node::WorkerThreadsTaskRunner>&, std::allocator_traits<std::allocator<node::WorkerThreadsTaskRunner> >::__construct_helper*, (node::WorkerThreadsTaskRunner<int&>&&)...) (__a=..., __p=0x3395bf0)
    at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/alloc_traits.h:253
#6  0x0000000000ea54d5 in std::allocator_traits<std::allocator<node::WorkerThreadsTaskRunner> >::construct<node::WorkerThreadsTaskRunner<int&> >(std::allocator<node::WorkerThreadsTaskRunner>&, node::WorkerThreadsTaskRunner<int&>*, (node::WorkerThreadsTaskRunner<int&>&&)...) (__a=..., 
    __p=0x3395bf0) at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/alloc_traits.h:399
#7  0x0000000000ea37aa in std::__shared_ptr<node::WorkerThreadsTaskRunner, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<node::WorkerThreadsTaskRunner>, int&> (
    this=0x7fffffffde30, __tag=..., __a=...)
    at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/shared_ptr_base.h:1124
#8  0x0000000000ea1692 in std::shared_ptr<node::WorkerThreadsTaskRunner>::shared_ptr<std::alloca---Type <return> to continue, or q <return> to quit---
tor<node::WorkerThreadsTaskRunner>, int&> (this=0x7fffffffde30, __tag=..., __a=...)
    at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/shared_ptr.h:316
#9  0x0000000000e9fe4e in std::allocate_shared<node::WorkerThreadsTaskRunner, std::allocator<node::WorkerThreadsTaskRunner>, int&> (__a=...)
    at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/shared_ptr.h:588
#10 0x0000000000e9e7fb in std::make_shared<node::WorkerThreadsTaskRunner, int&> ()
    at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/shared_ptr.h:604
#11 0x0000000000e9a18f in node::NodePlatform::NodePlatform (this=0x3395b00, 
    thread_pool_size=4, tracing_controller=0x3395920) at ../src/node_platform.cc:293
#12 0x0000000000dbbdcc in Initialize (this=0x3359680 <node::v8_platform>, thread_pool_size=4)
    at ../src/node.cc:239
#13 0x0000000000dc26d4 in node::InitializeV8Platform (thread_pool_size=4)
    at ../src/node.cc:1893
#14 0x0000000000dc2cd7 in node::Start (argc=1, argv=0x338b4d0) at ../src/node.cc:2122
#15 0x0000000001e13199 in main (argc=1, argv=0x7fffffffe048) at ../src/node_main.cc:126
(gdb)

    @addaleax - this is what I see, am I missing something?

  7. gireeshpunathil commented on Dec 13, 2018

    @gireeshpunathil
    gireeshpunathil
    on Dec 13, 2018
    MemberAuthor

    ok, apologies; those are normal worker threads created every time at the bootup. I got confused with worker_thread's thread; my bad.

  8. addaleax commented on Dec 13, 2018

    @addaleax
    addaleax
    on Dec 13, 2018
    Member

    @gireeshpunathil Okay, that clears it up :)

    I think worker_thread’s threads are affected as well, though…

  9. gireeshpunathil
    removed
    workerIssues and PRs related to the worker_threads module and Worker API.
    on Dec 13, 2018

  10. gireeshpunathil commented on Dec 13, 2018

    @gireeshpunathil
    gireeshpunathil
    on Dec 13, 2018
    MemberAuthor

    ok, I edited the description to remove worker module from the limelight. I was misguided by the word worker in the WorkerThreadsTaskRunner - that led to writing this test too.

    So just to clarify (for myself and others): now this test case only solves the purpose to easily recreate / pronounce the issue through many worker threads, but workers are not real culprits.

  11. joyeecheung commented on Dec 13, 2018

    @joyeecheung
    joyeecheung
    on Dec 13, 2018
    Member

    As far as I can tell, it’s only the native binding that’s loaded unconditionally (to figure out whether we’re in a worker or not during bootstrap).

    I believe that should be unnecessary - opened #25017

  12. gireeshpunathil commented on Dec 15, 2018

    @gireeshpunathil
    gireeshpunathil
    on Dec 15, 2018
    MemberAuthor

    Part of debugging #24921 I happened to run the entire CI with underscored exit replacing normal exit in Environment::Exit. The only test that fails in pseudo-tty/test-set-raw-mode-reset-process-exit . I am not advocating in favor of _exit as the favored fix, just stating it here.

  13. 58 remaining items

  14. gireeshpunathil
    added a commit that references this issue on Feb 8, 2019

    test: exit sequence sanity tests

    2ea2000
  15. addaleax
    added a commit that references this issue on Feb 8, 2019

    test: exit sequence sanity tests

    4dca3ab
  16. BethGriggs
    added a commit that references this issue on Feb 13, 2019

    src: dispose of V8 platform in `process.exit()`

    3c3f350
  17. BethGriggs
    added 2 commits that reference this issue on Mar 27, 2019

    src: dispose of V8 platform in `process.exit()`

    25ca9f1

    src: dispose of V8 platform in `process.exit()`

    f4be176
  18. BethGriggs
    added a commit that references this issue on Apr 17, 2019

    test: mark two tests as flaky in AIX

    90e0ae5
  19. BethGriggs
    added 2 commits that reference this issue on Apr 28, 2019

    test: mark two tests as flaky in AIX

    b8efc9d

    test: revoke flaky designation for tests

    df179d7
  20. BethGriggs
    added 2 commits that reference this issue on May 10, 2019

    test: mark two tests as flaky in AIX

    e33cf1d

    test: revoke flaky designation for tests

    29ed864
  21. MylesBorins
    added 2 commits that reference this issue on May 16, 2019

    test: mark two tests as flaky in AIX

    0fe72b8

    test: revoke flaky designation for tests

    fbf8e60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    c++Issues and PRs that require attention from people who are familiar with C++.confirmed-bugIssues and PRs for confirmed bugs.lib / srcIssues and PRs involving general changes in the lib/ or src/ directories.processIssues and PRs related to the process subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions