8000 Revert "embedding: make Stop() stop Workers" · nodejs/node@0a35d49 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0a35d49

Browse files
addaleaxrichardlau
addaleax
authored and
richardlau
committed
Revert "embedding: make Stop() stop Workers"
This reverts commit 037ac99. As flaky CI failures have revealed, this feature was implemented incorrectly. `stop_sub_worker_contexts()` needs to be called on the thread on which the `Environment` is currently running, it’s not thread-safe. The current API requires `Stop()` to be thread-safe, though. We could add a new API for this, but unless there’s demand, that’s probably not necessary as `FreeEnvironment()` will also stop Workers, which is commonly the next action on an `Environment` instance after having `Stop()` called on it. Refs: #32531 PR-URL: #32623 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
1 parent 1b4790b commit 0a35d49

File tree

5 files changed

+8
-9
lines changed
  • env.cc
  • env.h
  • node.cc
  • node.h

    • 5 files changed

    +8
    -9
    lines changed

    ‎src/api/environment.cc‎

    Lines changed: 2 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -712,7 +712,8 @@ ThreadId AllocateEnvironmentThreadId() {
    712712
    }
    713713

    714714
    void DefaultProcessExitHandler(Environment* env, int exit_code) {
    715-
    Stop(env);
    715+
    env->set_can_call_into_js(false);
    716+
    env->stop_sub_worker_contexts();
    716717
    DisposePlatform();
    717718
    uv_library_shutdown();
    718719
    exit(exit_code);

    ‎src/env.cc‎

    Lines changed: 1 addition & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -550,10 +550,9 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {
    550550
    }
    551551
    }
    552552

    553-
    void Environment::Stop() {
    553+
    void Environment::ExitEnv() {
    554554
    set_can_call_into_js(false);
    555555
    set_stopping(true);
    556-
    stop_sub_worker_contexts();
    557556
    isolate_->TerminateExecution();
    558557
    SetImmediateThreadsafe([](Environment* env) { uv_stop(env->event_loop()); });
    559558
    }

    ‎src/env.h‎

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -926,7 +926,7 @@ class Environment : public MemoryRetainer {
    926926
    void RegisterHandleCleanups();
    927927
    void CleanupHandles();
    928928
    void Exit(int code);
    929-
    void Stop();
    929+
    void ExitEnv();
    930930

    931931
    // Register clean-up cb to be called on environment destruction.
    932932
    inline void RegisterHandleCleanup(uv_handle_t* handle,

    ‎src/node.cc‎

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -1035,7 +1035,7 @@ int Start(int argc, char** argv) {
    10351035
    }
    10361036

    10371037
    int Stop(Environment* env) {
    1038-
    env->Stop();
    1038+
    env->ExitEnv();
    10391039
    return 0;
    10401040
    }
    10411041

    ‎src/node.h‎

    Lines changed: 3 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -218,8 +218,7 @@ class Environment;
    218218
    NODE_EXTERN int Start(int argc, char* argv[]);
    219219

    220220
    // Tear down Node.js while it is running (there are active handles
    221-
    // in the loop and / or actively executing JavaScript code). This also stops
    222-
    // all Workers that may have been started earlier.
    221+
    // in the loop and / or actively executing JavaScript code).
    223222
    NODE_EXTERN int Stop(Environment* env);
    224223

    225224
    // TODO(addaleax): Officially deprecate this and replace it with something
    @@ -469,8 +468,8 @@ NODE_EXTERN void FreeEnvironment(Environment* env);
    469468
    // It receives the Environment* instance and the exit code as arguments.
    470469
    // This could e.g. call Stop(env); in order to terminate execution and stop
    471470
    // the event loop.
    472-
    // The default handler calls Stop(), disposes of the global V8 platform
    473-
    // instance, if one is being used, and calls exit().
    471+
    // The default handler disposes of the global V8 platform instance, if one is
    472+
    // being used, and calls exit().
    474473
    NODE_EXTERN void SetProcessExitHandler(
    475474
    Environment* env,
    476475
    std::function<void(Environment*, int)>&& handler);

    0 commit comments

    Comments
     (0)
    0