corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1926722 - Refactoring: Make RemoteLazyInputStreamThread's lifecycle more sound. r=dom-storage-reviewers,asuth,xpcom-reviewers,nika

Browse Source 
In order to make things clearer, we:
- Use already_AddRefed for Get and GetOrCreate's return, ensuring the AddRef happens under the locked mutex and expecting that callers hold a strong reference until they need it.
- Make mThread const, such that there can never be a situation where we cannot use it as long as we have our RemoteLazyInputStreamThread instance.
- Add thread safety annotions for gRemoteLazyThreadMutex.

We can rely on mThread refusing to work after shutdown, in particular to dispatch runnables, such that we can just delegate most calls without additional checks.

We also remove the previous explicit observer implementation in favor of the simpler RunOnShutdown.

Differential Revision: https://phabricator.services.mozilla.com/D227036
This commit is contained in:
Jens Stutte
2024-11-07 06:18:52 +00:00
parent c36169737f
commit c24dce6f5b
5 changed files with 66 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
dom/file/ipc/RemoteLazyInputStream.cpp
View File

@@ -157,7 +157,8 @@ RemoteLazyInputStream::RemoteLazyInputStream(nsIInputStream* aStream)
static already_AddRefed<RemoteLazyInputStreamChild> BindChildActor( static already_AddRefed<RemoteLazyInputStreamChild> BindChildActor(
nsID aId, mozilla::ipc::Endpoint<PRemoteLazyInputStreamChild> aEndpoint) { nsID aId, mozilla::ipc::Endpoint<PRemoteLazyInputStreamChild> aEndpoint) {
auto* thread = RemoteLazyInputStreamThread::GetOrCreate(); RefPtr<RemoteLazyInputStreamThread> thread =
RemoteLazyInputStreamThread::GetOrCreate();
if (NS_WARN_IF(!thread)) { if (NS_WARN_IF(!thread)) {
return nullptr; return nullptr;
} }
@@ -803,7 +804,8 @@ void RemoteLazyInputStream::StreamNeeded() {
MOZ_LOG(gRemoteLazyStreamLog, LogLevel::Debug, MOZ_LOG(gRemoteLazyStreamLog, LogLevel::Debug,
("StreamNeeded %s", Describe().get())); ("StreamNeeded %s", Describe().get()));
auto* thread = RemoteLazyInputStreamThread::GetOrCreate(); RefPtr<RemoteLazyInputStreamThread> thread =
RemoteLazyInputStreamThread::GetOrCreate();
if (NS_WARN_IF(!thread)) { if (NS_WARN_IF(!thread)) {
return; return;
} }
@@ -1216,7 +1218,8 @@ RemoteLazyInputStream::AsyncLengthWait(nsIInputStreamLengthCallback* aCallback,
if (mActor) { if (mActor) {
if (aCallback) { if (aCallback) {
auto* thread = RemoteLazyInputStreamThread::GetOrCreate(); RefPtr<RemoteLazyInputStreamThread> thread =
RemoteLazyInputStreamThread::GetOrCreate();
if (NS_WARN_IF(!thread)) { if (NS_WARN_IF(!thread)) {
return NS_ERROR_ILLEGAL_DURING_SHUTDOWN; return NS_ERROR_ILLEGAL_DURING_SHUTDOWN;
} }
@@ -1324,7 +1327,8 @@ void RemoteLazyInputStream::IPCWrite(IPC::MessageWriter* aWriter) {
MOZ_ALWAYS_SUCCEEDS( MOZ_ALWAYS_SUCCEEDS(
PRemoteLazyInputStream::CreateEndpoints(&parentEp, &childEp)); PRemoteLazyInputStream::CreateEndpoints(&parentEp, &childEp));
auto* thread = RemoteLazyInputStreamThread::GetOrCreate(); RefPtr<RemoteLazyInputStreamThread> thread =
RemoteLazyInputStreamThread::GetOrCreate();
if (thread) { if (thread) {
thread->Dispatch(NS_NewRunnableFunction( thread->Dispatch(NS_NewRunnableFunction(
"RemoteLazyInputStreamChild::SendClone", "RemoteLazyInputStreamChild::SendClone",

2
dom/file/ipc/RemoteLazyInputStreamChild.cpp
View File

@@ -29,7 +29,7 @@ void RemoteLazyInputStreamChild::StreamConsumed() {
// When the count reaches zero, close the underlying actor. // When the count reaches zero, close the underlying actor.
if (count == 0) { if (count == 0) {
auto* t = RemoteLazyInputStreamThread::Get(); RefPtr<RemoteLazyInputStreamThread> t = RemoteLazyInputStreamThread::Get();
if (t) { if (t) {
t->Dispatch( t->Dispatch(
NS_NewRunnableFunction("RemoteLazyInputStreamChild::StreamConsumed", NS_NewRunnableFunction("RemoteLazyInputStreamChild::StreamConsumed",

149
dom/file/ipc/RemoteLazyInputStreamThread.cpp
View File

@@ -22,114 +22,65 @@ namespace mozilla {
namespace { namespace {
StaticMutex gRemoteLazyThreadMutex; StaticMutex gRemoteLazyThreadMutex;
StaticRefPtr<RemoteLazyInputStreamThread> gRemoteLazyThread; StaticRefPtr<RemoteLazyInputStreamThread> gRemoteLazyThread
MOZ_GUARDED_BY(gRemoteLazyThreadMutex);
class ThreadInitializeRunnable final : public Runnable {
public:
ThreadInitializeRunnable() : Runnable("dom::ThreadInitializeRunnable") {}
NS_IMETHOD
Run() override {
StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
MOZ_ASSERT(gRemoteLazyThread);
if (NS_WARN_IF(!gRemoteLazyThread->InitializeOnMainThread())) {
// RemoteLazyInputStreamThread::GetOrCreate might have handed out a
// pointer to our thread already at this point such that we cannot
// just do gRemoteLazyThread = nullptr; here.
MOZ_DIAGNOSTIC_ASSERT(
false, "Async gRemoteLazyThread->InitializeOnMainThread() failed.");
return NS_ERROR_FAILURE;
}
return NS_OK;
}
};
} // namespace } // namespace
NS_IMPL_ISUPPORTS(RemoteLazyInputStreamThread, nsIObserver, nsIEventTarget, NS_IMPL_ISUPPORTS(RemoteLazyInputStreamThread, nsIEventTarget,
nsISerialEventTarget, nsIDirectTaskDispatcher) nsISerialEventTarget, nsIDirectTaskDispatcher)
bool RLISThreadIsInOrBeyondShutdown() { /* static */
// ShutdownPhase::XPCOMShutdownThreads matches already_AddRefed<RemoteLazyInputStreamThread>
// obs->AddObserver(this, NS_XPCOM_SHUTDOWN_THREADS_OBSERVER_ID, false); RemoteLazyInputStreamThread::Get() {
return AppShutdown::IsInOrBeyond(ShutdownPhase::XPCOMShutdownThreads); StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
return do_AddRef(gRemoteLazyThread);
} }
/* static */ /* static */
RemoteLazyInputStreamThread* RemoteLazyInputStreamThread::Get() { already_AddRefed<RemoteLazyInputStreamThread>
if (RLISThreadIsInOrBeyondShutdown()) { RemoteLazyInputStreamThread::GetOrCreate() {
return nullptr;
}
StaticMutexAutoLock lock(gRemoteLazyThreadMutex); StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
return gRemoteLazyThread; if (AppShutdown::IsInOrBeyond(ShutdownPhase::XPCOMShutdownThreads)) {
}
/* static */
RemoteLazyInputStreamThread* RemoteLazyInputStreamThread::GetOrCreate() {
if (RLISThreadIsInOrBeyondShutdown()) {
return nullptr; return nullptr;
} }
StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
if (!gRemoteLazyThread) { if (!gRemoteLazyThread) {
gRemoteLazyThread = new RemoteLazyInputStreamThread();
if (!gRemoteLazyThread->Initialize()) {
gRemoteLazyThread = nullptr;
}
}
return gRemoteLazyThread;
}
bool RemoteLazyInputStreamThread::Initialize() {
nsCOMPtr<nsIThread> thread; nsCOMPtr<nsIThread> thread;
nsresult rv = NS_NewNamedThread("RemoteLzyStream", getter_AddRefs(thread)); nsresult rv = NS_NewNamedThread("RemoteLzyStream", getter_AddRefs(thread));
if (NS_WARN_IF(NS_FAILED(rv))) { if (NS_WARN_IF(NS_FAILED(rv))) {
return false; return nullptr;
} }
mThread = thread; gRemoteLazyThread =
new RemoteLazyInputStreamThread(WrapMovingNotNull(thread));
if (!NS_IsMainThread()) { // Dispatch to the main thread, which will set up a listener
RefPtr<Runnable> runnable = new ThreadInitializeRunnable(); // to shut down the thread during XPCOMShutdownThreads.
nsresult rv = SchedulerGroup::Dispatch(runnable.forget()); //
return !NS_WARN_IF(NS_FAILED(rv)); // We do this even if we're already on the main thread, as
} // if we're too late in shutdown, this will trigger the thread
// to shut down synchronously.
return InitializeOnMainThread(); NS_DispatchToMainThread(NS_NewRunnableFunction(
} "RemoteLazyInputStreamThread::MainThreadInit", [] {
RunOnShutdown(
bool RemoteLazyInputStreamThread::InitializeOnMainThread() { [] {
MOZ_ASSERT(NS_IsMainThread()); RefPtr<RemoteLazyInputStreamThread> rlis =
RemoteLazyInputStreamThread::Get();
nsCOMPtr<nsIObserverService> obs = services::GetObserverService(); // This is the only place supposed to ever null our reference.
if (NS_WARN_IF(!obs)) { MOZ_ASSERT(rlis);
return false; rlis->mThread->Shutdown();
}
nsresult rv =
obs->AddObserver(this, NS_XPCOM_SHUTDOWN_THREADS_OBSERVER_ID, false);
return !NS_WARN_IF(NS_FAILED(rv));
}
NS_IMETHODIMP
RemoteLazyInputStreamThread::Observe(nsISupports* aSubject, const char* aTopic,
const char16_t* aData) {
MOZ_ASSERT(!strcmp(aTopic, NS_XPCOM_SHUTDOWN_THREADS_OBSERVER_ID));
StaticMutexAutoLock lock(gRemoteLazyThreadMutex); StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
gRemoteLazyThread = nullptr;
if (mThread) { },
mThread->Shutdown(); ShutdownPhase::XPCOMShutdownThreads);
mThread = nullptr; }));
} }
gRemoteLazyThread = nullptr; return do_AddRef(gRemoteLazyThread);
return NS_OK;
} }
// nsIEventTarget // nsIEventTarget
@@ -147,25 +98,13 @@ RemoteLazyInputStreamThread::IsOnCurrentThread(bool* aRetval) {
NS_IMETHODIMP NS_IMETHODIMP
RemoteLazyInputStreamThread::Dispatch(already_AddRefed<nsIRunnable> aRunnable, RemoteLazyInputStreamThread::Dispatch(already_AddRefed<nsIRunnable> aRunnable,
uint32_t aFlags) { uint32_t aFlags) {
nsCOMPtr<nsIRunnable> runnable(aRunnable); return mThread->Dispatch(std::move(aRunnable), aFlags);
if (RLISThreadIsInOrBeyondShutdown()) {
// nsIEventTarget::Dispatch must leak the runnable if the dispatch fails.
Unused << runnable.forget();
return NS_ERROR_ILLEGAL_DURING_SHUTDOWN;
}
StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
return mThread->Dispatch(runnable.forget(), aFlags);
} }
NS_IMETHODIMP NS_IMETHODIMP
RemoteLazyInputStreamThread::DispatchFromScript(nsIRunnable* aRunnable, RemoteLazyInputStreamThread::DispatchFromScript(nsIRunnable* aRunnable,
uint32_t aFlags) { uint32_t aFlags) {
nsCOMPtr<nsIRunnable> runnable(aRunnable); return mThread->Dispatch(do_AddRef(aRunnable), aFlags);
return Dispatch(runnable.forget(), aFlags);
} }
NS_IMETHODIMP NS_IMETHODIMP
@@ -189,8 +128,6 @@ RemoteLazyInputStreamThread::DispatchDirectTask(
already_AddRefed<nsIRunnable> aRunnable) { already_AddRefed<nsIRunnable> aRunnable) {
nsCOMPtr<nsIRunnable> runnable(aRunnable); nsCOMPtr<nsIRunnable> runnable(aRunnable);
StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
nsCOMPtr<nsIDirectTaskDispatcher> dispatcher = do_QueryInterface(mThread); nsCOMPtr<nsIDirectTaskDispatcher> dispatcher = do_QueryInterface(mThread);
if (dispatcher) { if (dispatcher) {
@@ -201,8 +138,6 @@ RemoteLazyInputStreamThread::DispatchDirectTask(
} }
NS_IMETHODIMP RemoteLazyInputStreamThread::DrainDirectTasks() { NS_IMETHODIMP RemoteLazyInputStreamThread::DrainDirectTasks() {
StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
nsCOMPtr<nsIDirectTaskDispatcher> dispatcher = do_QueryInterface(mThread); nsCOMPtr<nsIDirectTaskDispatcher> dispatcher = do_QueryInterface(mThread);
if (dispatcher) { if (dispatcher) {
@@ -213,8 +148,6 @@ NS_IMETHODIMP RemoteLazyInputStreamThread::DrainDirectTasks() {
} }
NS_IMETHODIMP RemoteLazyInputStreamThread::HaveDirectTasks(bool* aValue) { NS_IMETHODIMP RemoteLazyInputStreamThread::HaveDirectTasks(bool* aValue) {
StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
nsCOMPtr<nsIDirectTaskDispatcher> dispatcher = do_QueryInterface(mThread); nsCOMPtr<nsIDirectTaskDispatcher> dispatcher = do_QueryInterface(mThread);
if (dispatcher) { if (dispatcher) {
@@ -225,12 +158,8 @@ NS_IMETHODIMP RemoteLazyInputStreamThread::HaveDirectTasks(bool* aValue) {
} }
bool IsOnDOMFileThread() { bool IsOnDOMFileThread() {
MOZ_ASSERT(!RLISThreadIsInOrBeyondShutdown()); RefPtr<RemoteLazyInputStreamThread> rlis = RemoteLazyInputStreamThread::Get();
return rlis && rlis->IsOnCurrentThread();
StaticMutexAutoLock lock(gRemoteLazyThreadMutex);
MOZ_ASSERT(gRemoteLazyThread);
return gRemoteLazyThread->IsOnCurrentThreadInfallible();
} }
void AssertIsOnDOMFileThread() { MOZ_ASSERT(IsOnDOMFileThread()); } void AssertIsOnDOMFileThread() { MOZ_ASSERT(IsOnDOMFileThread()); }

20
dom/file/ipc/RemoteLazyInputStreamThread.h
View File

@@ -19,28 +19,28 @@ namespace mozilla {
class RemoteLazyInputStreamChild; class RemoteLazyInputStreamChild;
// XXX Rename this class since it's used by LSNG too. // XXX Rename this class since it's used by LSNG too.
class RemoteLazyInputStreamThread final : public nsIObserver, class RemoteLazyInputStreamThread final : public nsISerialEventTarget,
public nsISerialEventTarget,
public nsIDirectTaskDispatcher { public nsIDirectTaskDispatcher {
public: public:
NS_DECL_THREADSAFE_ISUPPORTS NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSIOBSERVER NS_DECL_NSIEVENTTARGET_FULL
NS_DECL_NSIEVENTTARGET
NS_DECL_NSISERIALEVENTTARGET NS_DECL_NSISERIALEVENTTARGET
NS_DECL_NSIDIRECTTASKDISPATCHER NS_DECL_NSIDIRECTTASKDISPATCHER
static RemoteLazyInputStreamThread* Get(); explicit RemoteLazyInputStreamThread(
MovingNotNull<nsCOMPtr<nsIThread>> aThread)
: mThread(std::move(aThread)) {}
static RemoteLazyInputStreamThread* GetOrCreate(); static already_AddRefed<RemoteLazyInputStreamThread> Get();
bool Initialize(); static already_AddRefed<RemoteLazyInputStreamThread> GetOrCreate();
bool InitializeOnMainThread();
private: private:
~RemoteLazyInputStreamThread() = default; ~RemoteLazyInputStreamThread() = default;
nsCOMPtr<nsIThread> mThread; // As long as we can access gRemoteLazyThread, mThread remains a valid
// object. We rely on it failing on late dispatch after its shutdown.
const NotNull<nsCOMPtr<nsIThread>> mThread;
}; };
bool IsOnDOMFileThread(); bool IsOnDOMFileThread();

2
testing/web-platform/meta/IndexedDB/__dir__.ini
View File

@@ -1,3 +1,3 @@
prefs: [extensions.blocklist.enabled:false] prefs: [extensions.blocklist.enabled:false]
lsan-allowed: [Malloc, NS_NewCancelableRunnableFunction, NS_NewPipe2, PR_NewMonitor, mozilla::RemoteLazyInputStream::CloneWithRange, mozilla::RemoteLazyInputStreamThread::GetOrCreate, nsSegmentedBuffer::AppendNewSegment, Alloc, MakeUnique, nsThread::nsThread, NewPage, mozilla::Queue, mozilla::detail::EventQueueInternal, mozilla::ThreadEventQueue::PutEventInternal ]
leak-threshold: [default:51200, tab:51200] leak-threshold: [default:51200, tab:51200]
lsan-allowed: [Alloc, MakeUnique, Malloc, NS_NewCancelableRunnableFunction, NS_NewPipe2, NewPage, PR_NewMonitor, mozilla::RemoteLazyInputStream::CloneWithRange, mozilla::RemoteLazyInputStreamThread::GetOrCreate, nsSegmentedBuffer::AppendNewSegment, nsThread::nsThread]