| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 2b24059 commit c190a05
4 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -512,13 +512,6 @@ void FreeEnvironment(Environment* env) { | |||
| 512 | 512 | RunAtExit(env); | |
| 513 | 513 | } | |
| 514 | 514 | ||
| 515 | - // This call needs to be made while the `Environment` is still alive | ||
| 516 | - // because we assume that it is available for async tracking in the | ||
| 517 | - // NodePlatform implementation. | ||
| 518 | - MultiIsolatePlatform* platform = env->isolate_data()->platform(); | ||
| 519 | - if (platform != nullptr) | ||
| 520 | - platform->DrainTasks(isolate); | ||
| 521 | - | ||
| 522 | 515 | delete env; | |
| 523 | 516 | } | |
| 524 | 517 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -65,10 +65,24 @@ NodeMainInstance::~NodeMainInstance() { | |||
| 65 | 65 | if (isolate_params_ == nullptr) { | |
| 66 | 66 | return; | |
| 67 | 67 | } | |
| 68 | - // This should only be done on a main instance that owns its isolate. | ||
| 69 | - // IsolateData must be freed before UnregisterIsolate() is called. | ||
| 70 | - isolate_data_.reset(); | ||
| 71 | - platform_->UnregisterIsolate(isolate_); | ||
| 68 | + | ||
| 69 | + { | ||
| 70 | + #ifdef DEBUG | ||
| 71 | + // node::Environment has been disposed and no JavaScript Execution is | ||
| 72 | + // allowed at this point. | ||
| 73 | + // Create a scope to check that no JavaScript is executed in debug build | ||
| 74 | + // and proactively crash the process in the case JavaScript is being | ||
| 75 | + // executed. | ||
| 76 | + // Isolate::Dispose() must be invoked outside of this scope to avoid | ||
| 77 | + // use-after-free. | ||
| 78 | + Isolate::DisallowJavascriptExecutionScope disallow_js( | ||
| 79 | + isolate_, Isolate::DisallowJavascriptExecutionScope::CRASH_ON_FAILURE); | ||
| 80 | + #endif | ||
| 81 | + // This should only be done on a main instance that owns its isolate. | ||
| 82 | + // IsolateData must be freed before UnregisterIsolate() is called. | ||
| 83 | + isolate_data_.reset(); | ||
| 84 | + platform_->UnregisterIsolate(isolate_); | ||
| 85 | + } | ||
| 72 | 86 | isolate_->Dispose(); | |
| 73 | 87 | } | |
| 74 | 88 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -424,6 +424,11 @@ void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task) { | |||
| 424 | 424 | InternalCallbackScope::kNoFlags); | |
| 425 | 425 | task->Run(); | |
| 426 | 426 | } else { | |
| 427 | + // When the Environment was freed, the tasks of the Isolate should also be | ||
| 428 | + // canceled by `NodePlatform::UnregisterIsolate`. However, if the embedder | ||
| 429 | + // request to run the foreground task after the Environment was freed, run | ||
| 430 | + // the task without InternalCallbackScope. | ||
| 431 | + | ||
| 427 | 432 | // The task is moved out of InternalCallbackScope if env is not available. | |
| 428 | 433 | // This is a required else block, and should not be removed. | |
| 429 | 434 | // See comment: https://github.com/nodejs/node/pull/34688#pullrequestreview-463867489 | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,23 @@ | |||
| 1 | + // Flags: --expose-gc | ||
| 2 | + 'use strict'; | ||
| 3 | + const common = require('../common'); | ||
| 4 | + | ||
| 5 | + // This test verifies that when a V8 FinalizationRegistryCleanupTask is queue | ||
| 6 | + // at the last moment when JavaScript can be executed, the callback of a | ||
| 7 | + // FinalizationRegistry will not be invoked and the process should exit | ||
| 8 | + // normally. | ||
| 9 | + | ||
| 10 | + const reg = new FinalizationRegistry( | ||
| 11 | + common.mustNotCall('This FinalizationRegistry callback should never be called')); | ||
| 12 | + | ||
| 13 | + function register() { | ||
| 14 | + // Create a temporary object in a new function scope to allow it to be GC-ed. | ||
| 15 | + reg.register({}); | ||
| 16 | + } | ||
| 17 | + | ||
| 18 | + process.on('exit', () => { | ||
| 19 | + // This is the final chance to execute JavaScript. | ||
| 20 | + register(); | ||
| 21 | + // Queue a FinalizationRegistryCleanupTask by a testing gc request. | ||
| 22 | + global.gc(); | ||
| 23 | + }); | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments