[api] Rename v8::Locker::IsActive to v8::Locker::WasEverUsed

IsActive is misleading as the current implementation forces to use
v8::Locker for all Isolate access once any Locker has been used in
the same process.

Bug: chromium:1240851
Change-Id: Ieb2cfa352313b6f2cbec1bafdbc94a3fc718f3d6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3190093
Reviewed-by: Dan Elphick <delphick@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77243}
This commit is contained in:
Camillo Bruni 2021-09-28 12:18:48 +02:00 committed by V8 LUCI CQ
parent a1e6efd80c
commit d78d8b7d1c
6 changed files with 25 additions and 14 deletions

View File

@ -64,7 +64,7 @@ class Isolate;
* given thread. This can be useful if you have code that can be called either * given thread. This can be useful if you have code that can be called either
* from code that holds the lock or from code that does not. The Unlocker is * from code that holds the lock or from code that does not. The Unlocker is
* not recursive so you can not have several Unlockers on the stack at once, and * not recursive so you can not have several Unlockers on the stack at once, and
* you can not use an Unlocker in a thread that is not inside a Locker's scope. * you cannot use an Unlocker in a thread that is not inside a Locker's scope.
* *
* An unlocker will unlock several lockers if it has to and reinstate the * An unlocker will unlock several lockers if it has to and reinstate the
* correct depth of locking on its destruction, e.g.: * correct depth of locking on its destruction, e.g.:
@ -122,8 +122,13 @@ class V8_EXPORT Locker {
static bool IsLocked(Isolate* isolate); static bool IsLocked(Isolate* isolate);
/** /**
* Returns whether v8::Locker is being used by this V8 instance. * Returns whether any v8::Locker has ever been used in this process.
* TODO(cbruni, chromium:1240851): Fix locking checks on a per-thread basis.
* The current implementation is quite confusing and leads to unexpected
* results if anybody uses v8::Locker in the current process.
*/ */
static bool WasEverUsed();
V8_DEPRECATE_SOON("Use WasEverUsed instead")
static bool IsActive(); static bool IsActive();
// Disallow copying and assigning. // Disallow copying and assigning.

View File

@ -947,7 +947,7 @@ void HandleScope::Initialize(Isolate* isolate) {
// We make an exception if the serializer is enabled, which means that the // We make an exception if the serializer is enabled, which means that the
// Isolate is exclusively used to create a snapshot. // Isolate is exclusively used to create a snapshot.
Utils::ApiCheck( Utils::ApiCheck(
!v8::Locker::IsActive() || !v8::Locker::WasEverUsed() ||
internal_isolate->thread_manager()->IsLockedByCurrentThread() || internal_isolate->thread_manager()->IsLockedByCurrentThread() ||
internal_isolate->serializer_enabled(), internal_isolate->serializer_enabled(),
"HandleScope::HandleScope", "HandleScope::HandleScope",
@ -9036,7 +9036,7 @@ void Isolate::IsolateInBackgroundNotification() {
void Isolate::MemoryPressureNotification(MemoryPressureLevel level) { void Isolate::MemoryPressureNotification(MemoryPressureLevel level) {
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this); i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
bool on_isolate_thread = bool on_isolate_thread =
v8::Locker::IsActive() v8::Locker::WasEverUsed()
? isolate->thread_manager()->IsLockedByCurrentThread() ? isolate->thread_manager()->IsLockedByCurrentThread()
: i::ThreadId::Current() == isolate->thread_id(); : i::ThreadId::Current() == isolate->thread_id();
isolate->heap()->MemoryPressureNotification(level, on_isolate_thread); isolate->heap()->MemoryPressureNotification(level, on_isolate_thread);

View File

@ -20,7 +20,7 @@ namespace {
// Track whether this V8 instance has ever called v8::Locker. This allows the // Track whether this V8 instance has ever called v8::Locker. This allows the
// API code to verify that the lock is always held when V8 is being entered. // API code to verify that the lock is always held when V8 is being entered.
base::Atomic32 g_locker_was_ever_used_ = 0; base::AtomicWord g_locker_was_ever_used_ = 0;
} // namespace } // namespace
@ -53,8 +53,12 @@ bool Locker::IsLocked(v8::Isolate* isolate) {
return internal_isolate->thread_manager()->IsLockedByCurrentThread(); return internal_isolate->thread_manager()->IsLockedByCurrentThread();
} }
bool Locker::IsActive() { // static
return !!base::Relaxed_Load(&g_locker_was_ever_used_); bool Locker::IsActive() { return WasEverUsed(); }
// static
bool Locker::WasEverUsed() {
return base::Relaxed_Load(&g_locker_was_ever_used_) != 0;
} }
Locker::~Locker() { Locker::~Locker() {

View File

@ -944,9 +944,10 @@ class Ticker : public sampler::Sampler {
void SampleStack(const v8::RegisterState& state) override { void SampleStack(const v8::RegisterState& state) override {
if (!profiler_) return; if (!profiler_) return;
Isolate* isolate = reinterpret_cast<Isolate*>(this->isolate()); Isolate* isolate = reinterpret_cast<Isolate*>(this->isolate());
if (v8::Locker::IsActive() && (!isolate->thread_manager()->IsLockedByThread( if (v8::Locker::WasEverUsed() &&
perThreadData_->thread_id()) || (!isolate->thread_manager()->IsLockedByThread(
perThreadData_->thread_state() != nullptr)) perThreadData_->thread_id()) ||
perThreadData_->thread_state() != nullptr))
return; return;
TickSample sample; TickSample sample;
sample.Init(isolate, state, TickSample::kIncludeCEntryFrame, true); sample.Init(isolate, state, TickSample::kIncludeCEntryFrame, true);

View File

@ -40,9 +40,10 @@ class CpuSampler : public sampler::Sampler {
void SampleStack(const v8::RegisterState& regs) override { void SampleStack(const v8::RegisterState& regs) override {
Isolate* isolate = reinterpret_cast<Isolate*>(this->isolate()); Isolate* isolate = reinterpret_cast<Isolate*>(this->isolate());
if (v8::Locker::IsActive() && (!isolate->thread_manager()->IsLockedByThread( if (v8::Locker::WasEverUsed() &&
perThreadData_->thread_id()) || (!isolate->thread_manager()->IsLockedByThread(
perThreadData_->thread_state() != nullptr)) { perThreadData_->thread_id()) ||
perThreadData_->thread_state() != nullptr)) {
ProfilerStats::Instance()->AddReason( ProfilerStats::Instance()->AddReason(
ProfilerStats::Reason::kIsolateNotLocked); ProfilerStats::Reason::kIsolateNotLocked);
return; return;

View File

@ -122,7 +122,7 @@ void CcTest::Run() {
DCHECK_EQ(active_isolates, i::Isolate::non_disposed_isolates()); DCHECK_EQ(active_isolates, i::Isolate::non_disposed_isolates());
#endif // DEBUG #endif // DEBUG
if (initialize_) { if (initialize_) {
if (v8::Locker::IsActive()) { if (v8::Locker::WasEverUsed()) {
v8::Locker locker(isolate_); v8::Locker locker(isolate_);
EmptyMessageQueues(isolate_); EmptyMessageQueues(isolate_);
} else { } else {