Skip to content

Commit 78b9e17

Browse files
committed
Bug 1937908 - Block shutdown in GMPProcessParent. r=aosmond,media-playback-reviewers
There is a shutdown race that can happen in GMPParent. On ActorDestroy(NormalShutdown), if the GMP process is backlogged with work, it may not yet have shut down. At the same time, ActorDestroy(NormalShutdown) happens after xpcom-shutdown-threads, which has made sure to shut down the GMP event target in GMPService. The graceful shutdown of the GMP process requires dispatching tasks to the GMP event target (see GMPParent::DeleteProcess), which, if the target has been shut down, will result in the GMP process leaking because the SendShutdown Then handler could not be dispatched. If it even gets that far. Differential Revision: https://phabricator.services.mozilla.com/D234856
1 parent e94fd49 commit 78b9e17

File tree

3 files changed

+17
-3
lines changed

3 files changed

+17
-3
lines changed

dom/media/gmp/GMPParent.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -814,7 +814,8 @@ static void GMPNotifyObservers(const uint32_t aPluginID,
814814

815815
void GMPParent::ActorDestroy(ActorDestroyReason aWhy) {
816816
MOZ_ASSERT(GMPEventTarget()->IsOnCurrentThread());
817-
GMP_PARENT_LOG_DEBUG("%s: (%d)", __FUNCTION__, (int)aWhy);
817+
GMP_PARENT_LOG_DEBUG("%s: (%d), state=%u", __FUNCTION__, (int)aWhy,
818+
static_cast<GMPState>(mState));
818819

819820
if (AbnormalShutdown == aWhy) {
820821
Telemetry::Accumulate(Telemetry::SUBPROCESS_ABNORMAL_ABORT, "gmplugin"_ns,
@@ -839,7 +840,8 @@ void GMPParent::ActorDestroy(ActorDestroyReason aWhy) {
839840
mAbnormalShutdownInProgress = true;
840841
CloseActive(false);
841842

842-
// Normal Shutdown() will delete the process on unwind.
843+
// Normal Shutdown() will delete the process on unwind. GMPProcessParent
844+
// blocks shutdown to avoid races.
843845
if (AbnormalShutdown == aWhy) {
844846
RefPtr<GMPParent> self(this);
845847
// Must not call Close() again in DeleteProcess(), as we'll recurse

dom/media/gmp/GMPProcessParent.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "mozilla/ipc/ProcessChild.h"
1616
#include "mozilla/ipc/ProcessUtils.h"
1717
#include "mozilla/StaticPrefs_media.h"
18+
#include "nsFmtString.h"
1819

1920
#include "base/string_util.h"
2021
#include "base/process_util.h"
@@ -232,7 +233,14 @@ bool GMPProcessParent::Launch(int32_t aTimeoutMs) {
232233
// We need to wait until OnChannelConnected to clear the pref serializer, but
233234
// SyncLaunch will block until that is called, so we don't actually need to do
234235
// any overriding, and it only lives on the stack.
235-
return SyncLaunch(std::move(args), aTimeoutMs);
236+
bool launched = SyncLaunch(std::move(args), aTimeoutMs);
237+
if (launched) {
238+
nsFmtString name{FMT_STRING(u"GMPProcessParent {}"),
239+
static_cast<void*>(this)};
240+
mShutdownBlocker = media::ShutdownBlockingTicket::Create(
241+
name, NS_LITERAL_STRING_FROM_CSTRING(__FILE__), __LINE__);
242+
}
243+
return launched;
236244
}
237245

238246
void GMPProcessParent::Delete(nsCOMPtr<nsIRunnable> aCallback) {

dom/media/gmp/GMPProcessParent.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "base/file_path.h"
1313
#include "base/thread.h"
1414
#include "mozilla/ipc/GeckoChildProcessHost.h"
15+
#include "mozilla/media/MediaUtils.h"
1516
#include "nsIFile.h"
1617

1718
class nsIRunnable;
@@ -89,6 +90,9 @@ class GMPProcessParent final : public mozilla::ipc::GeckoChildProcessHost {
8990
# endif
9091
#endif
9192

93+
// Ticket for blocking shutdown while the process is live.
94+
UniquePtr<media::ShutdownBlockingTicket> mShutdownBlocker;
95+
9296
// For normalizing paths to be compatible with sandboxing.
9397
// We use normalized paths to generate the sandbox ruleset. Once
9498
// the sandbox has been started, resolving symlinks that point to

0 commit comments

Comments
 (0)