nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1932468 - Making EventSourceImpl::mWorkerRef accessing be thread-safe. r=dom-worker-reviewers,asuth a=pascalc

Browse source 
EventSourceImpl::mWorkerRef is not accessed on the Worker thread only.
When EventSourceImpl::Dispatch is called, it could be on any thread, so the race could happen if Worker is shuttdown at the same time.
This patch uses DataMutex to protect EventSourceImpl::mWorkerRef.

Differential Revision: https://phabricator.services.mozilla.com/D239515
This commit is contained in:
Eden Chuang 2025-02-26 08:02:02 +00:00
parent 8d7942934b
commit b4bc1a7836
1 changed files with 17 additions and 8 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

25
dom/base/EventSource.cpp
View file

@ -257,8 +257,9 @@ class EventSourceImpl final : public nsIObserver,
nsString mLastFieldValue; nsString mLastFieldValue;
// EventSourceImpl internal states. // EventSourceImpl internal states.
// WorkerRef to keep the worker alive. (accessed on worker thread only) // WorkerRef to keep the worker alive.
RefPtr<ThreadSafeWorkerRef> mWorkerRef; DataMutex<RefPtr<ThreadSafeWorkerRef>> mWorkerRef;
// Whether the window is frozen. May be set on main thread and read on target // Whether the window is frozen. May be set on main thread and read on target
// thread. // thread.
Atomic<bool> mFrozen; Atomic<bool> mFrozen;
@ -372,6 +373,7 @@ EventSourceImpl::EventSourceImpl(EventSource* aEventSource,
nsICookieJarSettings* aCookieJarSettings) nsICookieJarSettings* aCookieJarSettings)
: mReconnectionTime(0), : mReconnectionTime(0),
mStatus(PARSE_STATE_OFF), mStatus(PARSE_STATE_OFF),
mWorkerRef(nullptr, "EventSourceImpl::mWorkerRef"),
mFrozen(false), mFrozen(false),
mGoingToDispatchAllMessages(false), mGoingToDispatchAllMessages(false),
mIsMainThread(NS_IsMainThread()), mIsMainThread(NS_IsMainThread()),
@ -1206,7 +1208,7 @@ void EventSourceImpl::ResetDecoder() {
class CallRestartConnection final : public WorkerMainThreadRunnable { class CallRestartConnection final : public WorkerMainThreadRunnable {
public: public:
explicit CallRestartConnection(RefPtr<EventSourceImpl>&& aEventSourceImpl) explicit CallRestartConnection(RefPtr<EventSourceImpl>&& aEventSourceImpl)
: WorkerMainThreadRunnable(aEventSourceImpl->mWorkerRef->Private(), : WorkerMainThreadRunnable(GetCurrentThreadWorkerPrivate(),
"EventSource :: RestartConnection"_ns), "EventSource :: RestartConnection"_ns),
mESImpl(std::move(aEventSourceImpl)) { mESImpl(std::move(aEventSourceImpl)) {
mWorkerPrivate->AssertIsOnWorkerThread(); mWorkerPrivate->AssertIsOnWorkerThread();
@ -1873,7 +1875,8 @@ class WorkerRunnableDispatcher final : public WorkerThreadRunnable {
} // namespace } // namespace
bool EventSourceImpl::CreateWorkerRef(WorkerPrivate* aWorkerPrivate) { bool EventSourceImpl::CreateWorkerRef(WorkerPrivate* aWorkerPrivate) {
MOZ_ASSERT(!mWorkerRef); auto tsWorkerRef = mWorkerRef.Lock();
MOZ_ASSERT(!*tsWorkerRef);
MOZ_ASSERT(aWorkerPrivate); MOZ_ASSERT(aWorkerPrivate);
aWorkerPrivate->AssertIsOnWorkerThread(); aWorkerPrivate->AssertIsOnWorkerThread();
@ -1889,14 +1892,15 @@ bool EventSourceImpl::CreateWorkerRef(WorkerPrivate* aWorkerPrivate) {
return false; return false;
} }
mWorkerRef = new ThreadSafeWorkerRef(workerRef); *tsWorkerRef = new ThreadSafeWorkerRef(workerRef);
return true; return true;
} }
void EventSourceImpl::ReleaseWorkerRef() { void EventSourceImpl::ReleaseWorkerRef() {
MOZ_ASSERT(IsClosed()); MOZ_ASSERT(IsClosed());
MOZ_ASSERT(IsCurrentThreadRunningWorker()); MOZ_ASSERT(IsCurrentThreadRunningWorker());
mWorkerRef = nullptr; auto workerRef = mWorkerRef.Lock();
*workerRef = nullptr;
} }
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
@ -1925,10 +1929,15 @@ EventSourceImpl::Dispatch(already_AddRefed<nsIRunnable> aEvent,
// If the target is a worker, we have to use a custom WorkerRunnableDispatcher // If the target is a worker, we have to use a custom WorkerRunnableDispatcher
// runnable. // runnable.
auto workerRef = mWorkerRef.Lock();
// Return NS_OK if the worker has already shutdown
if (!*workerRef) {
return NS_OK;
}
RefPtr<WorkerRunnableDispatcher> event = new WorkerRunnableDispatcher( RefPtr<WorkerRunnableDispatcher> event = new WorkerRunnableDispatcher(
this, mWorkerRef->Private(), event_ref.forget()); this, (*workerRef)->Private(), event_ref.forget());
if (!event->Dispatch(mWorkerRef->Private())) { if (!event->Dispatch((*workerRef)->Private())) {
return NS_ERROR_FAILURE; return NS_ERROR_FAILURE;
} }
return NS_OK; return NS_OK;