Skip to content

src: refactor Environment::GetCurrent(isolate) usage #26376

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

Closed
addaleax wants to merge 5 commits into from
Closed

src: refactor Environment::GetCurrent(isolate) usage #26376

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3509d74
src: refactor `Environment::GetCurrent(isolate)` usage
addaleax Feb 24, 2019
d85419d
src: prefer to get `Environment` from `Context`
addaleax Feb 24, 2019
322041b
src: allow running tasks without `Environment`
addaleax Feb 24, 2019
6b766af
src: forbid handle allocations from Platform tasks
addaleax Feb 24, 2019
7f99bbb
fixup! src: prefer to get `Environment` from `Context`
addaleax Mar 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
src: forbid handle allocations from Platform tasks 
Platform tasks should have their own handle scopes, rather than
leak into outer ones.
  • Loading branch information
@addaleax
addaleax committed Mar 1, 2019
commit 6b766afc1c3806f84b314d80d49782f0b48077c8
8 changes: 8 additions & 0 deletions src/inspector/main_thread_interface.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,14 @@ void MainThreadInterface::DispatchMessages() {
MessageQueue::value_type task;
std::swap(dispatching_message_queue_.front(), task);
dispatching_message_queue_.pop_front();

// TODO(addaleax): The V8 inspector code currently sometimes allocates
// handles that leak to the outside scope, rendering a HandleScope here
// necessary. This handle scope can be removed/turned into a
// SealHandleScope once/if
// https://chromium-review.googlesource.com/c/v8/v8/+/1484304 makes it
// into our copy of V8, maybe guarded with #ifdef DEBUG if we want.
v8::HandleScope handle_scope(isolate_);
task->Call(this);
}
} while (had_messages);
Expand Down
6 changes: 4 additions & 2 deletions src/node_platform.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@

namespace node {

using v8::HandleScope;
using v8::Isolate;
using v8::Local;
using v8::Object;
using v8::Platform;
using v8::SealHandleScope;
using v8::Task;
using node::tracing::TracingController;

Expand Down Expand Up @@ -332,7 +332,9 @@ int NodePlatform::NumberOfWorkerThreads() {

void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task) {
Isolate* isolate = Isolate::GetCurrent();
HandleScope scope(isolate);
#ifdef DEBUG
SealHandleScope scope(isolate);
#endif
Environment* env = Environment::GetCurrent(isolate);
if (env != nullptr) {
InternalCallbackScope cb_scope(env, Local<Object>(), { 0, 0 },
Expand Down