0

FLEDGE: Try to really fix worklet utility exit crashes, again.

The synchronization for V8 shutdown wasn't quite working right since it
invoked the cleanup callback from ~AuctionV8Helper before the isolate
was shutdown, so it handled the case of GC from running things on the V8
thread (which what the previous reports were for), but not from it
happening from ~Isolate. This changes the done with cleanup callback to
happen after the isolate is destroyed.

Bug: 1339232
Change-Id: Iff7e17966352015c6ec753242a19d6ce2119cfe8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4599427
Reviewed-by: Russ Hamilton <behamilton@google.com>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1154587}
This commit is contained in:
Maks Orlovich
2023-06-07 20:38:01 +00:00
committed by Chromium LUCI CQ
parent 17a247f558
commit 28e4c2bc7a
2 changed files with 7 additions and 6 deletions
content/services/auction_worklet

@ -346,8 +346,8 @@ AuctionV8Helper::CreateTaskRunner() {
}
void AuctionV8Helper::SetDestroyedCallback(base::OnceClosure callback) {
DCHECK(!destroyed_callback_);
destroyed_callback_ = std::move(callback);
DCHECK(!destroyed_callback_run_);
destroyed_callback_run_.ReplaceClosure(std::move(callback));
}
v8::Local<v8::Context> AuctionV8Helper::CreateContext(
@ -829,8 +829,6 @@ AuctionV8Helper::~AuctionV8Helper() {
// destroyed before `devtools_agent_`.
if (devtools_agent_)
devtools_agent_->DestroySessions();
if (destroyed_callback_)
std::move(destroyed_callback_).Run();
}
void AuctionV8Helper::CreateIsolate() {

@ -11,6 +11,7 @@
#include <vector>
#include "base/containers/span.h"
#include "base/functional/callback_helpers.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/ref_counted.h"
#include "base/memory/ref_counted_delete_on_sequence.h"
@ -387,6 +388,10 @@ class CONTENT_EXPORT AuctionV8Helper
scoped_refptr<base::SequencedTaskRunner> v8_runner_;
scoped_refptr<base::SequencedTaskRunner> timer_task_runner_;
// This needs to be invoked after ~IsolateHolder to make sure that V8 is
// really shut down.
base::ScopedClosureRunner destroyed_callback_run_;
std::unique_ptr<gin::IsolateHolder> isolate_holder_
GUARDED_BY_CONTEXT(sequence_checker_);
v8::Global<v8::Context> scratch_context_
@ -414,8 +419,6 @@ class CONTENT_EXPORT AuctionV8Helper
std::unique_ptr<v8_inspector::V8Inspector> v8_inspector_
GUARDED_BY_CONTEXT(sequence_checker_);
base::OnceClosure destroyed_callback_;
SEQUENCE_CHECKER(sequence_checker_);
};