From 4ce1ddbb6482fb898700a84e0aa28036fe9dcc8a Mon Sep 17 00:00:00 2001 From: Eden Chuang Date: Tue, 29 Apr 2025 13:50:20 +0000 Subject: [PATCH] Bug 1961460 - Remove the assertion for target thread in RemoteWorkerDebuggerManagerChild::Constructor. r=dom-worker-reviewers,jstutte RemoteWorkerService::InitializeOnTargetThread could race with its shutdown So we can not use sRemoteWorkerService for thread correctness checking in RemoteWorkerDebuggerManagerChild creation during the initialization. This patch remove the assertions in the RemoteWorkerDebuggerManagerChild constructor and also moving the RemoteWorkerDebuggerManagerChild creation into RemoteWorkerService::InitializeOnTargetThread. RemoteWorkerService::InitializeOnTargetThread provides the thread correctness checking by using its mThread. Differential Revision: https://phabricator.services.mozilla.com/D247043 --- dom/workers/WorkerPrivate.cpp | 2 +- .../RemoteWorkerDebuggerManagerChild.cpp | 6 +---- .../remoteworkers/RemoteWorkerService.cpp | 25 +++++++++++++------ .../remoteworkers/RemoteWorkerService.h | 4 ++- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 4b822c00b044..1120390e4c5d 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -3160,7 +3160,7 @@ already_AddRefed WorkerPrivate::Constructor( // Create remote debugger endpoint here for child binding in // WorkerThreadPrimaryRunnable - worker->CreateRemoteDebuggerEndpoints(); + // worker->CreateRemoteDebuggerEndpoints(); if (!runtimeService->RegisterWorker(*worker)) { aRv.Throw(NS_ERROR_UNEXPECTED); diff --git a/dom/workers/remoteworkers/RemoteWorkerDebuggerManagerChild.cpp b/dom/workers/remoteworkers/RemoteWorkerDebuggerManagerChild.cpp index 528876da575b..868a44dced82 100644 --- a/dom/workers/remoteworkers/RemoteWorkerDebuggerManagerChild.cpp +++ b/dom/workers/remoteworkers/RemoteWorkerDebuggerManagerChild.cpp @@ -7,11 +7,7 @@ namespace mozilla::dom { -RemoteWorkerDebuggerManagerChild::RemoteWorkerDebuggerManagerChild() { - MOZ_ASSERT_DEBUG_OR_FUZZING( - RemoteWorkerService::Thread() && - RemoteWorkerService::Thread()->IsOnCurrentThread()); -} +RemoteWorkerDebuggerManagerChild::RemoteWorkerDebuggerManagerChild() {} RemoteWorkerDebuggerManagerChild::~RemoteWorkerDebuggerManagerChild() {} diff --git a/dom/workers/remoteworkers/RemoteWorkerService.cpp b/dom/workers/remoteworkers/RemoteWorkerService.cpp index 9de06e45b57c..9949dc28cc4f 100644 --- a/dom/workers/remoteworkers/RemoteWorkerService.cpp +++ b/dom/workers/remoteworkers/RemoteWorkerService.cpp @@ -275,11 +275,8 @@ nsresult RemoteWorkerService::InitializeOnMainThread( "InitializeThread", [self, endpoint = std::move(aEndpoint), debuggerChildEp = std::move(aDebuggerChildEp)]() mutable { - self->InitializeOnTargetThread(std::move(endpoint)); - - self->mDebuggerManagerChild = - MakeRefPtr(); - debuggerChildEp.Bind(self->mDebuggerManagerChild); + self->InitializeOnTargetThread(std::move(endpoint), + std::move(debuggerChildEp)); }); rv = mThread->Dispatch(r.forget(), NS_DISPATCH_NORMAL); @@ -298,18 +295,26 @@ RemoteWorkerService::RemoteWorkerService() RemoteWorkerService::~RemoteWorkerService() = default; void RemoteWorkerService::InitializeOnTargetThread( - mozilla::ipc::Endpoint aEndpoint) { + mozilla::ipc::Endpoint aEndpoint, + mozilla::ipc::Endpoint + aDebuggerMgrEndpoint) { MOZ_ASSERT(mThread); MOZ_ASSERT(mThread->IsOnCurrentThread()); + RefPtr debuggerManagerActor = + MakeRefPtr(); + if (NS_WARN_IF(!aDebuggerMgrEndpoint.Bind(debuggerManagerActor))) { + return; + } + RefPtr serviceActor = MakeAndAddRef(); if (NS_WARN_IF(!aEndpoint.Bind(serviceActor))) { return; } - // Now we are ready! - mActor = serviceActor; + mDebuggerManagerChild = std::move(debuggerManagerActor); + mActor = std::move(serviceActor); } void RemoteWorkerService::CloseActorOnTargetThread() { @@ -322,6 +327,10 @@ void RemoteWorkerService::CloseActorOnTargetThread() { mActor->Close(); mActor = nullptr; } + if (mDebuggerManagerChild) { + mDebuggerManagerChild->Close(); + mDebuggerManagerChild = nullptr; + } } NS_IMETHODIMP diff --git a/dom/workers/remoteworkers/RemoteWorkerService.h b/dom/workers/remoteworkers/RemoteWorkerService.h index 1429db4f15da..459b98a7e772 100644 --- a/dom/workers/remoteworkers/RemoteWorkerService.h +++ b/dom/workers/remoteworkers/RemoteWorkerService.h @@ -111,7 +111,9 @@ class RemoteWorkerService final : public nsIObserver { aDebuggerChildEp); void InitializeOnTargetThread( - mozilla::ipc::Endpoint aEndpoint); + mozilla::ipc::Endpoint aEndpoint, + mozilla::ipc::Endpoint + aDebuggerMgrEndpoint); void CloseActorOnTargetThread();