Bug 1268434 - Mutex-protect and check GMP abstract thread before uses - c?cpearce
GeckoMediaPluginService::mAbstractThread was not reset as expected from
ShutdownGMPThread, meaning it would retain a reference to the GMP thread, and
it would allow dispatch attempts to the GMP thread after shutdown.
Also mAbstractThread was not protected by a mutex (as mGMPThread was), which is
definitely needed now that it can be reset at shutdown time.
As its prefix implies, GetAbstractThread could return a nullptr, so it should
be checked before every use.
Note that this GetAbstractThread call (and its check) has been moved closer to
the start of functions using it, to avoid unnecessary and potentially invariant-
breaking partial work to take place when we can know in advance that it won't
fully succeed because the GMP thread is not available.
MozReview-Commit-ID: B1drOeM65hr
--- a/dom/media/gmp/GMPService.cpp
+++ b/dom/media/gmp/GMPService.cpp
@@ -315,16 +315,17 @@ void
GeckoMediaPluginService::ShutdownGMPThread()
{
LOGD(("%s::%s", __CLASS__, __FUNCTION__));
nsCOMPtr<nsIThread> gmpThread;
{
MutexAutoLock lock(mMutex);
mGMPThreadShutdown = true;
mGMPThread.swap(gmpThread);
+ mAbstractGMPThread = nullptr;
}
if (gmpThread) {
gmpThread->Shutdown();
}
}
nsresult
@@ -366,28 +367,29 @@ GeckoMediaPluginService::GetThread(nsITh
nsresult rv = NS_NewNamedThread("GMPThread", getter_AddRefs(mGMPThread));
if (NS_FAILED(rv)) {
return rv;
}
mAbstractGMPThread = AbstractThread::CreateXPCOMThreadWrapper(mGMPThread, false);
// Tell the thread to initialize plugins
- InitializePlugins();
+ InitializePlugins(mAbstractGMPThread.get());
}
nsCOMPtr<nsIThread> copy = mGMPThread;
copy.forget(aThread);
return NS_OK;
}
RefPtr<AbstractThread>
GeckoMediaPluginService::GetAbstractGMPThread()
{
+ MutexAutoLock lock(mMutex);
return mAbstractGMPThread;
}
class GetGMPContentParentForAudioDecoderDone : public GetGMPContentParentCallback
{
public:
explicit GetGMPContentParentForAudioDecoderDone(UniquePtr<GetGMPAudioDecoderCallback>&& aCallback)
: mCallback(Move(aCallback))
--- a/dom/media/gmp/GMPService.h
+++ b/dom/media/gmp/GMPService.h
@@ -78,28 +78,28 @@ public:
RefPtr<AbstractThread> GetAbstractGMPThread();
protected:
GeckoMediaPluginService();
virtual ~GeckoMediaPluginService();
void RemoveObsoletePluginCrashCallbacks(); // Called from add/run.
- virtual void InitializePlugins() = 0;
+ virtual void InitializePlugins(AbstractThread* aAbstractGMPThread) = 0;
virtual bool GetContentParentFrom(const nsACString& aNodeId,
const nsCString& aAPI,
const nsTArray<nsCString>& aTags,
UniquePtr<GetGMPContentParentCallback>&& aCallback) = 0;
nsresult GMPDispatch(nsIRunnable* event, uint32_t flags = NS_DISPATCH_NORMAL);
nsresult GMPDispatch(already_AddRefed<nsIRunnable> event, uint32_t flags = NS_DISPATCH_NORMAL);
void ShutdownGMPThread();
- Mutex mMutex; // Protects mGMPThread and mGMPThreadShutdown and some members
- // in derived classes.
+ Mutex mMutex; // Protects mGMPThread, mAbstractGMPThread and
+ // mGMPThreadShutdown and some members in derived classes.
nsCOMPtr<nsIThread> mGMPThread;
RefPtr<AbstractThread> mAbstractGMPThread;
bool mGMPThreadShutdown;
bool mShuttingDownOnGMPThread;
class GMPCrashCallback
{
public:
--- a/dom/media/gmp/GMPServiceChild.h
+++ b/dom/media/gmp/GMPServiceChild.h
@@ -53,17 +53,17 @@ public:
NS_DECL_NSIOBSERVER
void SetServiceChild(UniquePtr<GMPServiceChild>&& aServiceChild);
void RemoveGMPContentParent(GMPContentParent* aGMPContentParent);
protected:
- void InitializePlugins() override
+ void InitializePlugins(AbstractThread*) override
{
// Nothing to do here.
}
bool GetContentParentFrom(const nsACString& aNodeId,
const nsCString& aAPI,
const nsTArray<nsCString>& aTags,
UniquePtr<GetGMPContentParentCallback>&& aCallback)
override;
--- a/dom/media/gmp/GMPServiceParent.cpp
+++ b/dom/media/gmp/GMPServiceParent.cpp
@@ -523,18 +523,22 @@ GeckoMediaPluginServiceParent::EnsureIni
}
bool
GeckoMediaPluginServiceParent::GetContentParentFrom(const nsACString& aNodeId,
const nsCString& aAPI,
const nsTArray<nsCString>& aTags,
UniquePtr<GetGMPContentParentCallback>&& aCallback)
{
+ RefPtr<AbstractThread> thread(GetAbstractGMPThread());
+ if (!thread) {
+ return false;
+ }
+
RefPtr<GeckoMediaPluginServiceParent> self(this);
- RefPtr<AbstractThread> thread(GetAbstractGMPThread());
nsCString nodeId(aNodeId);
nsTArray<nsCString> tags(aTags);
nsCString api(aAPI);
GetGMPContentParentCallback* rawCallback = aCallback.release();
EnsureInitialized()->Then(thread, __func__,
[self, tags, api, nodeId, rawCallback]() -> void {
UniquePtr<GetGMPContentParentCallback> callback(rawCallback);
RefPtr<GMPParent> gmp = self->SelectPluginForAPI(nodeId, api, tags);
@@ -550,28 +554,30 @@ GeckoMediaPluginServiceParent::GetConten
UniquePtr<GetGMPContentParentCallback> callback(rawCallback);
NS_WARNING("GMPService::EnsureInitialized failed.");
callback->Done(nullptr);
});
return true;
}
void
-GeckoMediaPluginServiceParent::InitializePlugins()
+GeckoMediaPluginServiceParent::InitializePlugins(
+ AbstractThread* aAbstractGMPThread)
{
+ MOZ_ASSERT(aAbstractGMPThread);
MonitorAutoLock lock(mInitPromiseMonitor);
if (mLoadPluginsFromDiskComplete) {
return;
}
RefPtr<GeckoMediaPluginServiceParent> self(this);
RefPtr<GenericPromise> p = mInitPromise.Ensure(__func__);
- RefPtr<AbstractThread> thread(GetAbstractGMPThread());
- InvokeAsync(thread, this, __func__, &GeckoMediaPluginServiceParent::LoadFromEnvironment)
- ->Then(thread, __func__,
+ InvokeAsync(aAbstractGMPThread, this, __func__,
+ &GeckoMediaPluginServiceParent::LoadFromEnvironment)
+ ->Then(aAbstractGMPThread, __func__,
[self]() -> void {
MonitorAutoLock lock(self->mInitPromiseMonitor);
self->mLoadPluginsFromDiskComplete = true;
self->mInitPromise.Resolve(true, __func__);
},
[self]() -> void {
MonitorAutoLock lock(self->mInitPromiseMonitor);
self->mLoadPluginsFromDiskComplete = true;
@@ -753,16 +759,20 @@ GeckoMediaPluginServiceParent::CrashPlug
mPlugins[i]->Crash();
}
}
RefPtr<GenericPromise::AllPromiseType>
GeckoMediaPluginServiceParent::LoadFromEnvironment()
{
MOZ_ASSERT(NS_GetCurrentThread() == mGMPThread);
+ RefPtr<AbstractThread> thread(GetAbstractGMPThread());
+ if (!thread) {
+ return GenericPromise::AllPromiseType::CreateAndReject(NS_ERROR_FAILURE, __func__);
+ }
const char* env = PR_GetEnv("MOZ_GMP_PATH");
if (!env || !*env) {
return GenericPromise::AllPromiseType::CreateAndResolve(true, __func__);
}
nsString allpaths;
if (NS_WARN_IF(NS_FAILED(NS_CopyNativeToUnicode(nsDependentCString(env), allpaths)))) {
@@ -780,17 +790,16 @@ GeckoMediaPluginServiceParent::LoadFromE
break;
} else {
promises.AppendElement(AddOnGMPThread(nsString(Substring(allpaths, pos, next - pos))));
pos = next + 1;
}
}
mScannedPluginOnDisk = true;
- RefPtr<AbstractThread> thread(GetAbstractGMPThread());
return GenericPromise::All(thread, promises);
}
class NotifyObserversTask final : public mozilla::Runnable {
public:
explicit NotifyObserversTask(const char* aTopic, nsString aData = EmptyString())
: mTopic(aTopic)
, mData(aData)
@@ -828,16 +837,20 @@ GeckoMediaPluginServiceParent::PathRunna
#endif
return NS_OK;
}
RefPtr<GenericPromise>
GeckoMediaPluginServiceParent::AsyncAddPluginDirectory(const nsAString& aDirectory)
{
RefPtr<AbstractThread> thread(GetAbstractGMPThread());
+ if (!thread) {
+ return GenericPromise::CreateAndReject(NS_ERROR_FAILURE, __func__);
+ }
+
nsString dir(aDirectory);
return InvokeAsync(thread, this, __func__, &GeckoMediaPluginServiceParent::AddOnGMPThread, dir)
->Then(AbstractThread::MainThread(), __func__,
[dir]() -> void {
LOGD(("GeckoMediaPluginServiceParent::AsyncAddPluginDirectory %s succeeded",
NS_ConvertUTF16toUTF8(dir).get()));
MOZ_ASSERT(NS_IsMainThread());
// For e10s, we must fire a notification so that all ContentParents notify
@@ -1056,33 +1069,37 @@ GeckoMediaPluginServiceParent::ClonePlug
return gmp.forget();
}
RefPtr<GenericPromise>
GeckoMediaPluginServiceParent::AddOnGMPThread(nsString aDirectory)
{
MOZ_ASSERT(NS_GetCurrentThread() == mGMPThread);
- LOGD(("%s::%s: %s", __CLASS__, __FUNCTION__, NS_LossyConvertUTF16toASCII(aDirectory).get()));
+ nsCString dir = NS_ConvertUTF16toUTF8(aDirectory);
+ RefPtr<AbstractThread> thread(GetAbstractGMPThread());
+ if (!thread) {
+ LOGD(("%s::%s: %s No GMP Thread", __CLASS__, __FUNCTION__, dir.get()));
+ return GenericPromise::CreateAndReject(NS_ERROR_FAILURE, __func__);
+ }
+ LOGD(("%s::%s: %s", __CLASS__, __FUNCTION__, dir.get()));
nsCOMPtr<nsIFile> directory;
nsresult rv = NS_NewLocalFile(aDirectory, false, getter_AddRefs(directory));
if (NS_WARN_IF(NS_FAILED(rv))) {
return GenericPromise::CreateAndReject(NS_ERROR_FAILURE, __func__);
}
RefPtr<GMPParent> gmp = CreateGMPParent();
if (!gmp) {
NS_WARNING("Can't Create GMPParent");
return GenericPromise::CreateAndReject(NS_ERROR_FAILURE, __func__);
}
RefPtr<GeckoMediaPluginServiceParent> self(this);
- RefPtr<AbstractThread> thread(GetAbstractGMPThread());
- nsCString dir = NS_ConvertUTF16toUTF8(aDirectory);
return gmp->Init(this, directory)->Then(thread, __func__,
[gmp, self, dir]() -> void {
LOGD(("%s::%s: %s Succeeded", __CLASS__, __FUNCTION__, dir.get()));
{
MutexAutoLock lock(self->mMutex);
self->mPlugins.AppendElement(gmp);
}
NS_DispatchToMainThread(new NotifyObserversTask("gmp-path-added"), NS_DISPATCH_NORMAL);
--- a/dom/media/gmp/GMPServiceParent.h
+++ b/dom/media/gmp/GMPServiceParent.h
@@ -107,17 +107,17 @@ private:
DirectoryFilter& aFilter);
void ForgetThisSiteOnGMPThread(const nsACString& aOrigin);
void ClearRecentHistoryOnGMPThread(PRTime aSince);
protected:
friend class GMPParent;
void ReAddOnGMPThread(const RefPtr<GMPParent>& aOld);
void PluginTerminated(const RefPtr<GMPParent>& aOld);
- void InitializePlugins() override;
+ void InitializePlugins(AbstractThread* aAbstractGMPThread) override;
RefPtr<GenericPromise::AllPromiseType> LoadFromEnvironment();
RefPtr<GenericPromise> AddOnGMPThread(nsString aDirectory);
bool GetContentParentFrom(const nsACString& aNodeId,
const nsCString& aAPI,
const nsTArray<nsCString>& aTags,
UniquePtr<GetGMPContentParentCallback>&& aCallback)
override;
private:
--- a/dom/media/gtest/TestGMPRemoveAndDelete.cpp
+++ b/dom/media/gtest/TestGMPRemoveAndDelete.cpp
@@ -230,16 +230,17 @@ GMPRemoveTest::Setup()
GeneratePlugin();
GetService()->GetThread(getter_AddRefs(mGMPThread));
// Spin the event loop until the GMP service has had a chance to complete
// adding GMPs from MOZ_GMP_PATH. Otherwise, the RemovePluginDirectory()
// below may complete before we're finished adding GMPs from MOZ_GMP_PATH,
// and we'll end up not removing the GMP, and the test will fail.
RefPtr<AbstractThread> thread(GetServiceParent()->GetAbstractGMPThread());
+ EXPECT_TRUE(thread);
GMPTestMonitor* mon = &mTestMonitor;
GetServiceParent()->EnsureInitialized()->Then(thread, __func__,
[mon]() { mon->SetFinished(); },
[mon]() { mon->SetFinished(); }
);
mTestMonitor.AwaitFinished();
nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();