Skip to content

Commit

Permalink
Revert "src: unregister Isolate with platform before disposing"
Browse files Browse the repository at this point in the history
This reverts commit 65e5a8a.

PR-URL: #31782
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
  • Loading branch information
MylesBorins committed Feb 14, 2020
1 parent f0b2d87 commit d130184
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 9 deletions.
1 change: 0 additions & 1 deletion src/node.h
Expand Up @@ -280,7 +280,6 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {

// This function may only be called once per `Isolate`, and discard any
// pending delayed tasks scheduled for that isolate.
// This needs to be called right before calling `Isolate::Dispose()`.
virtual void UnregisterIsolate(v8::Isolate* isolate) = 0;

// The platform should call the passed function once all state associated
Expand Down
2 changes: 1 addition & 1 deletion src/node_main_instance.cc
Expand Up @@ -99,8 +99,8 @@ NodeMainInstance::~NodeMainInstance() {
if (!owns_isolate_) {
return;
}
platform_->UnregisterIsolate(isolate_);
isolate_->Dispose();
platform_->UnregisterIsolate(isolate_);
}

int NodeMainInstance::Run() {
Expand Down
7 changes: 1 addition & 6 deletions src/node_worker.cc
Expand Up @@ -192,13 +192,8 @@ class WorkerThreadData {
*static_cast<bool*>(data) = true;
}, &platform_finished);

// The order of these calls is important; if the Isolate is first disposed
// and then unregistered, there is a race condition window in which no
// new Isolate at the same address can successfully be registered with
// the platform.
// (Refs: https://github.com/nodejs/node/issues/30846)
w_->platform_->UnregisterIsolate(isolate);
isolate->Dispose();
w_->platform_->UnregisterIsolate(isolate);

// Wait until the platform has cleaned up all relevant resources.
while (!platform_finished)
Expand Down
2 changes: 1 addition & 1 deletion test/cctest/node_test_fixture.h
Expand Up @@ -106,9 +106,9 @@ class NodeTestFixture : public ::testing::Test {

void TearDown() override {
isolate_->Exit();
isolate_->Dispose();
platform->DrainTasks(isolate_);
platform->UnregisterIsolate(isolate_);
isolate_->Dispose();
isolate_ = nullptr;
}
};
Expand Down

0 comments on commit d130184

Please sign in to comment.