0

[gin] Enter the isolate before deleting a context

Deleting the context writes to the external pointer table and to the
heap, hence we must enter the isolate before deleting the context.

This is blocking a new DCHECK in v8: https://crrev.com/c/6387098

For consistency, we should also move the HandleScope from the
`PerContextData` destructor to the `ShellRunner` destructor, but this
causes failures in other users of `PerContextData`, so this is left for
a follow-up CL.

Drive-by cleanup: Remove `using` declarations which were mostly not
used, and if they were then inconsistently.

R=mlippautz@chromium.org

Bug: 396607238
Change-Id: I110a72c047d3a719e9606fc98f32f2b0536bb656
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6394240
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1438234}
This commit is contained in:
Clemens Backes
2025-03-26 10:30:51 -07:00
committed by Chromium LUCI CQ
parent de01803179
commit b272e34517

@ -13,23 +13,16 @@
#include "gin/try_catch.h"
#include "v8/include/v8-script.h"
using v8::Context;
using v8::HandleScope;
using v8::Isolate;
using v8::Object;
using v8::ObjectTemplate;
using v8::Script;
namespace gin {
ShellRunnerDelegate::ShellRunnerDelegate() = default;
ShellRunnerDelegate::~ShellRunnerDelegate() = default;
v8::Local<ObjectTemplate> ShellRunnerDelegate::GetGlobalTemplate(
v8::Local<v8::ObjectTemplate> ShellRunnerDelegate::GetGlobalTemplate(
ShellRunner* runner,
v8::Isolate* isolate) {
return v8::Local<ObjectTemplate>();
return v8::Local<v8::ObjectTemplate>();
}
void ShellRunnerDelegate::DidCreateContext(ShellRunner* runner) {
@ -46,12 +39,12 @@ void ShellRunnerDelegate::UnhandledException(ShellRunner* runner,
NOTREACHED() << try_catch.GetStackTrace();
}
ShellRunner::ShellRunner(ShellRunnerDelegate* delegate, Isolate* isolate)
ShellRunner::ShellRunner(ShellRunnerDelegate* delegate, v8::Isolate* isolate)
: delegate_(delegate) {
v8::Isolate::Scope isolate_scope(isolate);
HandleScope handle_scope(isolate);
v8::Local<v8::Context> context =
Context::New(isolate, NULL, delegate_->GetGlobalTemplate(this, isolate));
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Context> context = v8::Context::New(
isolate, nullptr, delegate_->GetGlobalTemplate(this, isolate));
context_holder_ = std::make_unique<ContextHolder>(isolate);
context_holder_->SetContext(context);
@ -61,16 +54,22 @@ ShellRunner::ShellRunner(ShellRunnerDelegate* delegate, Isolate* isolate)
delegate_->DidCreateContext(this);
}
ShellRunner::~ShellRunner() = default;
ShellRunner::~ShellRunner() {
// Deleting the ContextRunner deletes the contained PerContextData, which
// writes to the V8 heap in its destructor. Hence enter the isolate before
// doing that.
v8::Isolate::Scope isolate_scope(GetContextHolder()->isolate());
context_holder_.reset();
}
v8::MaybeLocal<v8::Value> ShellRunner::Run(const std::string& source,
const std::string& resource_name) {
v8::Isolate* isolate = GetContextHolder()->isolate();
TryCatch try_catch(isolate);
v8::ScriptOrigin origin(StringToV8(isolate, resource_name));
auto maybe_script = Script::Compile(GetContextHolder()->context(),
StringToV8(isolate, source), &origin);
v8::Local<Script> script;
auto maybe_script = v8::Script::Compile(GetContextHolder()->context(),
StringToV8(isolate, source), &origin);
v8::Local<v8::Script> script;
if (!maybe_script.ToLocal(&script)) {
delegate_->UnhandledException(this, try_catch);
return v8::MaybeLocal<v8::Value>();
@ -83,7 +82,7 @@ ContextHolder* ShellRunner::GetContextHolder() {
return context_holder_.get();
}
v8::MaybeLocal<v8::Value> ShellRunner::Run(v8::Local<Script> script) {
v8::MaybeLocal<v8::Value> ShellRunner::Run(v8::Local<v8::Script> script) {
TryCatch try_catch(GetContextHolder()->isolate());
delegate_->WillRunScript(this);