Skip to content

Assertion `!flush_tasks_' failed in parallel/test-worker-memory #30850

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.

Sign up for GitHub

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

Jump to bottom
Closed
bnoordhuis opened this issue Dec 8, 2019 · 2 comments
Closed

Assertion `!flush_tasks_' failed in parallel/test-worker-memory #30850

bnoordhuis opened this issue Dec 8, 2019 · 2 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. duplicate Issues and PRs that are duplicates of other issues or PRs. v8 platform Issues and PRs related to Node's v8::Platform implementation.

Comments

@bnoordhuis
Copy link
Member

Today's master, commit a9abc17:

=== release test-worker-memory ===
Path: parallel/test-worker-memory
run() called with n=15 (numWorkers=4)
run() called with n=15 (numWorkers=4)
run() called with n=15 (numWorkers=4)
run() called with n=15 (numWorkers=4)
run() called with n=14 (numWorkers=4)
run() called with n=14 (numWorkers=4)
run() called with n=14 (numWorkers=4)
run() called with n=14 (numWorkers=4)
run() called with n=13 (numWorkers=4)
run() called with n=13 (numWorkers=4)
run() called with n=13 (numWorkers=4)
run() called with n=12 (numWorkers=4)
run() called with n=12 (numWorkers=4)
run() called with n=13 (numWorkers=4)
run() called with n=11 (numWorkers=4)
run() called with n=11 (numWorkers=4)
run() called with n=12 (numWorkers=4)
run() called with n=10 (numWorkers=4)
run() called with n=10 (numWorkers=4)
run() called with n=11 (numWorkers=4)
run() called with n=12 (numWorkers=4)
run() called with n=9 (numWorkers=4)
run() called with n=9 (numWorkers=4)
run() called with n=10 (numWorkers=4)
run() called with n=8 (numWorkers=4)
run() called with n=11 (numWorkers=4)
run() called with n=8 (numWorkers=4)
out/Release/node[20609]: ../src/node_platform.cc:275:virtual node::PerIsolatePlatformData::~PerIsolatePlatformData(): Assertion `!flush_tasks_' failed.
 1: 0x10007f7b8 node::Abort() [/Users/bnoordhuis/src/v1.x/out/Release/node]
 2: 0x10007f565 node::AppendExceptionLine(node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Message>, node::ErrorHandlingMode) [/Users/bnoordhuis/src/v1.x/out/Release/node]
 3: 0x1000cfaa3 node::PerIsolatePlatformData::~PerIsolatePlatformData() [/Users/bnoordhuis/src/v1.x/out/Release/node]
 4: 0x1000cfe7b node::NodePlatform::RegisterIsolate(v8::Isolate*, uv_loop_s*) [/Users/bnoordhuis/src/v1.x/out/Release/node]
 5: 0x1000f2d6b node::worker::WorkerThreadData::WorkerThreadData(node::worker::Worker*) [/Users/bnoordhuis/src/v1.x/out/Release/node]
 6: 0x1000f0a4c node::worker::Worker::Run() [/Users/bnoordhuis/src/v1.x/out/Release/node]
 7: 0x1000f32ae node::worker::Worker::StartThread(v8::FunctionCallbackInfo<v8::Value> const&)::$_2::__invoke(void*) [/Users/bnoordhuis/src/v1.x/out/Release/node]
 8: 0x7fff7cf412eb _pthread_body [/usr/lib/system/libsystem_pthread.dylib]
 9: 0x7fff7cf44249 _pthread_start [/usr/lib/system/libsystem_pthread.dylib]
10: 0x7fff7cf4040d thread_start [/usr/lib/system/libsystem_pthread.dylib]
Command: out/Release/node /Users/bnoordhuis/src/v1.x/test/parallel/test-worker-memory.js
--- CRASHED (Signal: 6) ---

Presumably a race condition because I've only seen it happen once. I ran the test in a loop for a while but no dice.

#30324 probably either introduced or surfaced this. (It introduced the check but it's possible the errant behavior existed before that. Also that CHECK should probably be CHECK_NULL.)

cc @addaleax @laverdet
@bnoordhuis bnoordhuis added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 8, 2019
@addaleax
Copy link
Member

addaleax commented Dec 8, 2019

Also, duplicate of #30846 I guess? Did this happen locally for you?

@bnoordhuis
Copy link
Member Author

Oh, I searched for flush_tasks_ but that didn't turn up anything. Yes, it happened locally.

I'll close this out in favor of #30846.

@bnoordhuis bnoordhuis added the duplicate Issues and PRs that are duplicates of other issues or PRs. label Dec 8, 2019
@addaleax addaleax mentioned this issue Dec 11, 2019
Closed
@addaleax addaleax added the v8 platform Issues and PRs related to Node's v8::Platform implementation. label Feb 18, 2020
This was referenced Apr 13, 2021
Open
Open
Open
Open
Open
Open
Open
Open
This was referenced Sep 17, 2021
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
@Dmarch28 Dmarch28 mentioned this issue May 13, 2022
Open
@aliceUnhinged613 aliceUnhinged613 mentioned this issue May 13, 2022
Open
@137717unity 137717unity mentioned this issue May 13, 2022
Open
@baby636 baby636 mentioned this issue May 13, 2022
Open
@ron-a20 ron-a20 mentioned this issue May 13, 2022
Open
@molllyn1 molllyn1 mentioned this issue May 14, 2022
Open
@snyk-bot snyk-bot mentioned this issue Sep 25, 2022
Open
@wedataintelligence wedataintelligence mentioned this issue Sep 26, 2022
Open
@137717unity 137717unity mentioned this issue Sep 26, 2022
Open
@snyk-bot snyk-bot mentioned this issue Sep 27, 2022
Open
@wedataintelligence wedataintelligence mentioned this issue Sep 29, 2022
Open
This was referenced Sep 29, 2022
Open
Open
This was referenced Oct 5, 2022
Open
Open
@snyk-bot snyk-bot mentioned this issue Oct 6, 2022
Open
@137717unity 137717unity mentioned this issue Oct 7, 2022
Open
@wedataintelligence wedataintelligence mentioned this issue Oct 7, 2022
Open
This was referenced Oct 8, 2022
Open
Open
@137717unity 137717unity mentioned this issue Oct 9, 2022
Open
@snyk-bot snyk-bot mentioned this issue Oct 9, 2022
Open
@aliceUnhinged613 aliceUnhinged613 mentioned this issue Nov 25, 2023
Open
@wedataintelligence wedataintelligence mentioned this issue Nov 28, 2023
Open
@baby636 baby636 mentioned this issue Nov 28, 2023
Open
@joyeecheung joyeecheung mentioned this issue Apr 21, 2025
Open
joyeecheung added a commit to joyeecheung/node that referenced this issue Apr 22, 2025
@joyeecheung
deps: V8: backport f4107cff50a4
22eadd2 
Original commit message:

    [api] add Isolate::Free() and IsolateDisposeFlags::kDontFree

    This allows embedders to mirror the isolate disposal routine
    with an initialization routine that uses Isolate::Allocate().

    ```
    v8::Isolate* isolate = v8::Isolate::Allocate();
    // Use the isolate address as a key.
    v8::Isolate::Initialize(isolate, params);

    isolate->Dispose(v8::Isolate::IsolateDisposeFlags::kDontFree);
    // Remove the entry keyed by isolate address.
    v8::Isolate::Free(isolate);
    ```

    Previously, the only way to dispose the isolate bundles the
    de-initialization and the freeing of the address together in
    v8::Isolate::Dispose(). This is inadequate for embedders like
    Node.js that uses the isolate address as a key to manage the
    task runner associated with it, if another thread gets an
    isolate allocated at the aligned address before the other
    thread finishes cleanup for the isolate previously allocated
    at the same address, and locking on the entire disposal can
    be too risky since it may post GC tasks that in turn requires
    using the isolate address to locate the task runner. It's a
    lot simpler to handle the issue if the disposal process of
    the isolate can mirror the initialization of it and split
    into two routines.

    Refs: nodejs#57753 (comment)
    Refs: nodejs#30850

Refs: v8/v8@f4107cf
joyeecheung added a commit to joyeecheung/node that referenced this issue Apr 22, 2025
@joyeecheung
deps: V8: backport f4107cff50a4
88fdc61 
Original commit message:

    [api] add Isolate::Deinitialize() and Isolate::Free()

    This allows embedders to mirror the isolate disposal routine
    with an initialization routine that uses Isolate::Allocate().

    ```
    v8::Isolate* isolate = v8::Isolate::Allocate();
    // Use the isolate address as a key.
    v8::Isolate::Initialize(isolate, params);

    isolate->Deinitialize();
    // Remove the entry keyed by isolate address.
    v8::Isolate::Free(isolate);
    ```

    Previously, the only way to dispose the isolate bundles the
    de-initialization and the freeing of the address together in
    v8::Isolate::Dispose(). This is inadequate for embedders like
    Node.js that uses the isolate address as a key to manage the
    task runner associated with it, if another thread gets an
    isolate allocated at the aligned address before the other
    thread finishes cleanup for the isolate previously allocated
    at the same address, and locking on the entire disposal can
    be too risky since it may post GC tasks that in turn requires
    using the isolate address to locate the task runner. It's a
    lot simpler to handle the issue if the disposal process of
    the isolate can mirror the initialization of it and split
    into two routines.

    Refs: nodejs#57753 (comment)
    Refs: nodejs#30850

Refs: v8/v8@f4107cf
joyeecheung added a commit to joyeecheung/node that referenced this issue Apr 24, 2025
@joyeecheung
deps: V8: backport 75289aadf9c8
f301a23 
Original commit message:

    add Isolate::Deinitialize() and Isolate::Free()

    This allows embedders to mirror the isolate disposal routine
    with an initialization routine that uses Isolate::Allocate().

    ```
    v8::Isolate* isolate = v8::Isolate::Allocate();
    // Use the isolate address as a key.
    v8::Isolate::Initialize(isolate, params);

    isolate->Deinitialize();
    // Remove the entry keyed by isolate address.
    v8::Isolate::Free(isolate);
    ```

    Previously, the only way to dispose the isolate bundles the
    de-initialization and the freeing of the address together in
    v8::Isolate::Dispose(). This is inadequate for embedders like
    Node.js that uses the isolate address as a key to manage the
    task runner associated with it, if another thread gets an
    isolate allocated at the aligned address before the other
    thread finishes cleanup for the isolate previously allocated
    at the same address, and locking on the entire disposal can
    be too risky since it may post GC tasks that in turn requires
    using the isolate address to locate the task runner. It's a
    lot simpler to handle the issue if the disposal process of
    the isolate can mirror the initialization of it and split
    into two routines.

    Refs: nodejs#57753 (comment)
    Refs: nodejs#30850
    Bug: 412943769
    Change-Id: I3865c27395aded3a6f32de74d96d0698b2d891b9

Refs: v8/v8@75289aa
hubot pushed a commit to v8/v8 that referenced this issue Apr 24, 2025
@joyeecheung
[api] add Isolate::Deinitialize() and Isolate::Free()
954187b 
This allows embedders to mirror the isolate disposal routine
with an initialization routine that uses Isolate::Allocate().

```
v8::Isolate* isolate = v8::Isolate::Allocate();
// Use the isolate address as a key.
v8::Isolate::Initialize(isolate, params);

isolate->Deinitialize();
// Remove the entry keyed by isolate address.
v8::Isolate::Free(isolate);
```

Previously, the only way to dispose the isolate bundles the
de-initialization and the freeing of the address together in
v8::Isolate::Dispose(). This is inadequate for embedders like
Node.js that uses the isolate address as a key to manage the
task runner associated with it, if another thread gets an
isolate allocated at the aligned address before the other
thread finishes cleanup for the isolate previously allocated
at the same address, and locking on the entire disposal can
be too risky since it may post GC tasks that in turn requires
using the isolate address to locate the task runner. It's a
lot simpler to handle the issue if the disposal process of
the isolate can mirror the initialization of it and split
into two routines.

Refs: nodejs/node#57753 (comment)
Refs: nodejs/node#30850
Bug: 412943769
Change-Id: I3865c27395aded3a6f32de74d96d0698b2d891b9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6480071
Reviewed-by: Leszek Swirski <[email protected]>
Commit-Queue: Joyee Cheung <[email protected]>
Cr-Commit-Position: refs/heads/main@{#99890}
joyeecheung added a commit to joyeecheung/node that referenced this issue Apr 24, 2025
@joyeecheung
deps: V8: backport 954187bb1b87
4b04d69 
Original commit message:

    [api] add Isolate::Deinitialize() and Isolate::Free()

    This allows embedders to mirror the isolate disposal routine
    with an initialization routine that uses Isolate::Allocate().

    ```
    v8::Isolate* isolate = v8::Isolate::Allocate();
    // Use the isolate address as a key.
    v8::Isolate::Initialize(isolate, params);

    isolate->Deinitialize();
    // Remove the entry keyed by isolate address.
    v8::Isolate::Free(isolate);
    ```

    Previously, the only way to dispose the isolate bundles the
    de-initialization and the freeing of the address together in
    v8::Isolate::Dispose(). This is inadequate for embedders like
    Node.js that uses the isolate address as a key to manage the
    task runner associated with it, if another thread gets an
    isolate allocated at the aligned address before the other
    thread finishes cleanup for the isolate previously allocated
    at the same address, and locking on the entire disposal can
    be too risky since it may post GC tasks that in turn requires
    using the isolate address to locate the task runner. It's a
    lot simpler to handle the issue if the disposal process of
    the isolate can mirror the initialization of it and split
    into two routines.

    Refs: nodejs#57753 (comment)
    Refs: nodejs#30850
    Bug: 412943769
    Change-Id: I3865c27395aded3a6f32de74d96d0698b2d891b9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6480071
    Reviewed-by: Leszek Swirski <[email protected]>
    Commit-Queue: Joyee Cheung <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#99890}

Refs: v8/v8@954187b
joyeecheung added a commit to targos/node that referenced this issue Apr 25, 2025
@joyeecheung
deps: V8: backport 954187bb1b87
1285490 
Original commit message:

    [api] add Isolate::Deinitialize() and Isolate::Free()

    This allows embedders to mirror the isolate disposal routine
    with an initialization routine that uses Isolate::Allocate().

    ```
    v8::Isolate* isolate = v8::Isolate::Allocate();
    // Use the isolate address as a key.
    v8::Isolate::Initialize(isolate, params);

    isolate->Deinitialize();
    // Remove the entry keyed by isolate address.
    v8::Isolate::Free(isolate);
    ```

    Previously, the only way to dispose the isolate bundles the
    de-initialization and the freeing of the address together in
    v8::Isolate::Dispose(). This is inadequate for embedders like
    Node.js that uses the isolate address as a key to manage the
    task runner associated with it, if another thread gets an
    isolate allocated at the aligned address before the other
    thread finishes cleanup for the isolate previously allocated
    at the same address, and locking on the entire disposal can
    be too risky since it may post GC tasks that in turn requires
    using the isolate address to locate the task runner. It's a
    lot simpler to handle the issue if the disposal process of
    the isolate can mirror the initialization of it and split
    into two routines.

    Refs: nodejs#57753 (comment)
    Refs: nodejs#30850
    Bug: 412943769
    Change-Id: I3865c27395aded3a6f32de74d96d0698b2d891b9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6480071
    Reviewed-by: Leszek Swirski <[email protected]>
    Commit-Queue: Joyee Cheung <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#99890}

Refs: v8/v8@954187b
joyeecheung added a commit to targos/node that referenced this issue Apr 28, 2025
@joyeecheung
deps: V8: backport 954187bb1b87
698020f 
Original commit message:

    [api] add Isolate::Deinitialize() and Isolate::Free()

    This allows embedders to mirror the isolate disposal routine
    with an initialization routine that uses Isolate::Allocate().

    ```
    v8::Isolate* isolate = v8::Isolate::Allocate();
    // Use the isolate address as a key.
    v8::Isolate::Initialize(isolate, params);

    isolate->Deinitialize();
    // Remove the entry keyed by isolate address.
    v8::Isolate::Free(isolate);
    ```

    Previously, the only way to dispose the isolate bundles the
    de-initialization and the freeing of the address together in
    v8::Isolate::Dispose(). This is inadequate for embedders like
    Node.js that uses the isolate address as a key to manage the
    task runner associated with it, if another thread gets an
    isolate allocated at the aligned address before the other
    thread finishes cleanup for the isolate previously allocated
    at the same address, and locking on the entire disposal can
    be too risky since it may post GC tasks that in turn requires
    using the isolate address to locate the task runner. It's a
    lot simpler to handle the issue if the disposal process of
    the isolate can mirror the initialization of it and split
    into two routines.

    Refs: nodejs#57753 (comment)
    Refs: nodejs#30850
    Bug: 412943769
    Change-Id: I3865c27395aded3a6f32de74d96d0698b2d891b9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6480071
    Reviewed-by: Leszek Swirski <[email protected]>
    Commit-Queue: Joyee Cheung <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#99890}

Refs: v8/v8@954187b
targos pushed a commit to targos/node that referenced this issue Apr 29, 2025
@joyeecheung @targos
deps: V8: backport 954187bb1b87
1ee4371 
Original commit message:

    [api] add Isolate::Deinitialize() and Isolate::Free()

    This allows embedders to mirror the isolate disposal routine
    with an initialization routine that uses Isolate::Allocate().

    ```
    v8::Isolate* isolate = v8::Isolate::Allocate();
    // Use the isolate address as a key.
    v8::Isolate::Initialize(isolate, params);

    isolate->Deinitialize();
    // Remove the entry keyed by isolate address.
    v8::Isolate::Free(isolate);
    ```

    Previously, the only way to dispose the isolate bundles the
    de-initialization and the freeing of the address together in
    v8::Isolate::Dispose(). This is inadequate for embedders like
    Node.js that uses the isolate address as a key to manage the
    task runner associated with it, if another thread gets an
    isolate allocated at the aligned address before the other
    thread finishes cleanup for the isolate previously allocated
    at the same address, and locking on the entire disposal can
    be too risky since it may post GC tasks that in turn requires
    using the isolate address to locate the task runner. It's a
    lot simpler to handle the issue if the disposal process of
    the isolate can mirror the initialization of it and split
    into two routines.

    Refs: nodejs#57753 (comment)
    Refs: nodejs#30850
    Bug: 412943769
    Change-Id: I3865c27395aded3a6f32de74d96d0698b2d891b9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6480071
    Reviewed-by: Leszek Swirski <[email protected]>
    Commit-Queue: Joyee Cheung <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#99890}

Refs: v8/v8@954187b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. duplicate Issues and PRs that are duplicates of other issues or PRs. v8 platform Issues and PRs related to Node's v8::Platform implementation.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bnoordhuis @addaleax