Skip to content

Commit e8d3dab

Browse files
fix(worker): leave the inspector pause loop when the worker is terminating
A breakpoint hit while console.log converts its arguments pauses the worker inside ConsoleLog, which holds the worker's inspector mutex. terminate() could only end that pause through NotifyTerminating, which takes the same mutex, so the parent's terminate() blocked until the debugger resumed. The pause loop now also leaves on the worker's own termination flag, which terminate() sets before it takes the mutex.
1 parent 44f3c8c commit e8d3dab

3 files changed

Lines changed: 16 additions & 7 deletions

File tree

‎test-app/runtime/src/main/cpp/WorkerInspectorClient.cpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,15 @@ std::string ToUtf8String(const StringView& view) {
3434
} // namespace
3535

3636
WorkerInspectorClient::WorkerInspectorClient(int workerId, Isolate* isolate, ALooper* workerLooper,
37-
const std::string& url)
37+
const std::string& url,
38+
const std::atomic_bool& workerTerminating)
3839
: workerId_(workerId),
3940
sessionId_("NS_WORKER_" + std::to_string(workerId)),
4041
targetId_("ns-worker-" + std::to_string(workerId)),
4142
url_(url),
4243
isolate_(isolate),
43-
workerLooper_(workerLooper) {
44+
workerLooper_(workerLooper),
45+
workerTerminating_(workerTerminating) {
4446
// Wakes the worker looper when CDP messages arrive on the socket thread;
4547
// same mechanism as the worker's message inbox (ConcurrentQueue).
4648
eventFd_ = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
@@ -206,13 +208,13 @@ void WorkerInspectorClient::MaybeResetSession() {
206208
}
207209

208210
void WorkerInspectorClient::runMessageLoopOnPause(int contextGroupId) {
209-
if (runningPauseLoop_.load(std::memory_order_acquire) || dying_) {
211+
if (runningPauseLoop_.load(std::memory_order_acquire) || dying_ || workerTerminating_) {
210212
return;
211213
}
212214
runningPauseLoop_.store(true, std::memory_order_release);
213215
pauseTerminated_ = false;
214216

215-
while (!pauseTerminated_ && !dying_) {
217+
while (!pauseTerminated_ && !dying_ && !workerTerminating_) {
216218
std::string message = this->PopMessage();
217219
bool shouldWait = message.empty();
218220
if (!shouldWait) {
@@ -225,7 +227,7 @@ void WorkerInspectorClient::runMessageLoopOnPause(int contextGroupId) {
225227
->GetEventLoop(isolate_)
226228
->RunNestableV8Tasks();
227229

228-
if (shouldWait && !pauseTerminated_ && !dying_) {
230+
if (shouldWait && !pauseTerminated_ && !dying_ && !workerTerminating_) {
229231
std::unique_lock<std::mutex> lock(messageArrivedMutex_);
230232
messageArrived_.wait_for(lock, std::chrono::milliseconds(1));
231233
}

‎test-app/runtime/src/main/cpp/WorkerInspectorClient.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,10 @@ class WorkerInspectorClient final : public v8_inspector::V8InspectorClient,
3535
public v8_inspector::V8Inspector::Channel {
3636
public:
3737
// Worker thread, with the worker isolate locked and its context created.
38+
// `workerTerminating` is the worker's own termination flag; the
39+
// worker outlives this client.
3840
WorkerInspectorClient(int workerId, v8::Isolate* isolate, ALooper* workerLooper,
39-
const std::string& url);
41+
const std::string& url, const std::atomic_bool& workerTerminating);
4042
~WorkerInspectorClient() override;
4143

4244
int WorkerId() const {
@@ -126,6 +128,10 @@ class WorkerInspectorClient final : public v8_inspector::V8InspectorClient,
126128

127129
std::atomic<bool> dying_{false};
128130
std::atomic<bool> pauseTerminated_{false};
131+
// Read by the pause loop, which leaves on it without NotifyTerminating:
132+
// a pause entered while the worker holds its inspector mutex, inside
133+
// console.log, would otherwise keep Terminate() waiting for that mutex.
134+
const std::atomic_bool& workerTerminating_;
129135
std::atomic<bool> runningPauseLoop_{false}; // written on the worker thread only
130136
bool pendingReset_ = false; // worker thread only
131137
};

‎test-app/runtime/src/main/cpp/WorkerWrapper.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -818,7 +818,8 @@ void WorkerWrapper::CreateInspector(Isolate* isolate) {
818818
? workerPath_
819819
: "file://" + workerPath_;
820820

821-
auto* client = new WorkerInspectorClient(workerId_, isolate, ALooper_forThread(), url);
821+
auto* client = new WorkerInspectorClient(workerId_, isolate, ALooper_forThread(), url,
822+
isTerminating_);
822823
{
823824
std::lock_guard<std::mutex> lock(inspectorMutex_);
824825
inspector_ = client;

0 commit comments

Comments
 (0)