From 1f8c395df4a3f6833dfac01c1d27009329b13306 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Wed, 5 Feb 2025 14:05:24 -0500 Subject: [PATCH 1/3] inspector: make dispatching_messages_ atomic --- src/inspector/main_thread_interface.cc | 43 ++++++++++++++------------ src/inspector/main_thread_interface.h | 2 +- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/inspector/main_thread_interface.cc b/src/inspector/main_thread_interface.cc index 0fca7483e95c65..5039c04af9d465 100644 --- a/src/inspector/main_thread_interface.cc +++ b/src/inspector/main_thread_interface.cc @@ -240,26 +240,29 @@ void MainThreadInterface::StopWaitingForFrontendEvent() { } void MainThreadInterface::DispatchMessages() { - if (dispatching_messages_) - return; - dispatching_messages_ = true; - bool had_messages = false; - do { - if (dispatching_message_queue_.empty()) { - Mutex::ScopedLock scoped_lock(requests_lock_); - requests_.swap(dispatching_message_queue_); - } - had_messages = !dispatching_message_queue_.empty(); - while (!dispatching_message_queue_.empty()) { - MessageQueue::value_type task; - std::swap(dispatching_message_queue_.front(), task); - dispatching_message_queue_.pop_front(); - - v8::SealHandleScope seal_handle_scope(agent_->env()->isolate()); - task->Call(this); - } - } while (had_messages); - dispatching_messages_ = false; + // std::memory_order_acquire ensures that all writes made before this atomic exchange + // (by other threads) are visible to the current thread. + // If another thread set dispatching_messages_ to false and then performed some other actions, + // the current thread is guaranteed to see those other actions before it proceeds. + if (!dispatching_messages_.exchange(true, std::memory_order_acquire)) { + bool had_messages = false; + do { + if (dispatching_message_queue_.empty()) { + Mutex::ScopedLock scoped_lock(requests_lock_); + requests_.swap(dispatching_message_queue_); + } + had_messages = !dispatching_message_queue_.empty(); + while (!dispatching_message_queue_.empty()) { + MessageQueue::value_type task; + std::swap(dispatching_message_queue_.front(), task); + dispatching_message_queue_.pop_front(); + + v8::SealHandleScope seal_handle_scope(agent_->env()->isolate()); + task->Call(this); + } + } while (had_messages); + dispatching_messages_ = false; + } } std::shared_ptr MainThreadInterface::GetHandle() { diff --git a/src/inspector/main_thread_interface.h b/src/inspector/main_thread_interface.h index 35997adbe5ac66..2b7adeaa6b0b1a 100644 --- a/src/inspector/main_thread_interface.h +++ b/src/inspector/main_thread_interface.h @@ -94,7 +94,7 @@ class MainThreadInterface : // This queue is to maintain the order of the messages for the cases // when we reenter the DispatchMessages function. MessageQueue dispatching_message_queue_; - bool dispatching_messages_ = false; + std::atomic dispatching_messages_ = false; // This flag indicates an internal request to exit the loop in // WaitForFrontendEvent(). It's set to true by calling // StopWaitingForFrontendEvent(). From fcf67504d0a77e7219f8bdd9a3e993518a1c72a4 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Wed, 5 Feb 2025 14:34:45 -0500 Subject: [PATCH 2/3] lint --- src/inspector/main_thread_interface.cc | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/inspector/main_thread_interface.cc b/src/inspector/main_thread_interface.cc index 5039c04af9d465..ec353807bb8c27 100644 --- a/src/inspector/main_thread_interface.cc +++ b/src/inspector/main_thread_interface.cc @@ -240,11 +240,10 @@ void MainThreadInterface::StopWaitingForFrontendEvent() { } void MainThreadInterface::DispatchMessages() { - // std::memory_order_acquire ensures that all writes made before this atomic exchange - // (by other threads) are visible to the current thread. - // If another thread set dispatching_messages_ to false and then performed some other actions, - // the current thread is guaranteed to see those other actions before it proceeds. - if (!dispatching_messages_.exchange(true, std::memory_order_acquire)) { + bool expected = false; + // compare_exchange_strong returns true if the value was successfully changed + // from false to true. + if (dispatching_messages_.compare_exchange_strong(expected, true, std::memory_order_acquire, std::memory_order_relaxed)) { bool had_messages = false; do { if (dispatching_message_queue_.empty()) { @@ -261,7 +260,7 @@ void MainThreadInterface::DispatchMessages() { task->Call(this); } } while (had_messages); - dispatching_messages_ = false; + dispatching_messages_.store(false, std::memory_order_release); } } From 0b317a34da5eb676430cf78dec90b58cba26f91b Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Wed, 5 Feb 2025 14:34:57 -0500 Subject: [PATCH 3/3] lint --- src/inspector/main_thread_interface.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/inspector/main_thread_interface.cc b/src/inspector/main_thread_interface.cc index ec353807bb8c27..bcdf890d143d80 100644 --- a/src/inspector/main_thread_interface.cc +++ b/src/inspector/main_thread_interface.cc @@ -243,7 +243,11 @@ void MainThreadInterface::DispatchMessages() { bool expected = false; // compare_exchange_strong returns true if the value was successfully changed // from false to true. - if (dispatching_messages_.compare_exchange_strong(expected, true, std::memory_order_acquire, std::memory_order_relaxed)) { + if (dispatching_messages_.compare_exchange_strong( + expected, + true, + std::memory_order_acquire, + std::memory_order_relaxed)) { bool had_messages = false; do { if (dispatching_message_queue_.empty()) {