b=865241 don't join HRTFDatabaseLoader thread until it has finished r=ehsan
authorKarl Tomlinson <karlt+@karlt.net>
Fri, 09 Aug 2013 10:07:42 +1200
changeset 153778 c02414b48056f93938993e63db8e638f91397b2f
parent 153777 b085cd4bd4cad752405777968990dd048ee27acf
child 153779 beab9c07c4e4da8a7119058625a6275f09546ac2
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs865241
milestone25.0a2
b=865241 don't join HRTFDatabaseLoader thread until it has finished r=ehsan Also turn some always-true conditions into assertions. (transplanted from 7ab5c4babe56d9c00954d9f503b132afea499d26)
content/media/webaudio/WebAudioUtils.cpp
content/media/webaudio/WebAudioUtils.h
content/media/webaudio/blink/HRTFDatabaseLoader.cpp
content/media/webaudio/blink/HRTFDatabaseLoader.h
layout/build/nsLayoutStatics.cpp
--- a/content/media/webaudio/WebAudioUtils.cpp
+++ b/content/media/webaudio/WebAudioUtils.cpp
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim:set ts=2 sw=2 sts=2 et cindent: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WebAudioUtils.h"
 #include "AudioNodeStream.h"
+#include "blink/HRTFDatabaseLoader.h"
 
 namespace mozilla {
 
 namespace dom {
 
 struct ConvertTimeToTickHelper
 {
   AudioNodeStream* mSourceStream;
@@ -56,10 +57,16 @@ WebAudioUtils::ConvertAudioParamToTicks(
 {
   MOZ_ASSERT(!aSource || aSource->SampleRate() == aDest->SampleRate());
   ConvertTimeToTickHelper ctth;
   ctth.mSourceStream = aSource;
   ctth.mDestinationStream = aDest;
   aParam.ConvertEventTimesToTicks(ConvertTimeToTickHelper::Convert, &ctth, aDest->SampleRate());
 }
 
+void
+WebAudioUtils::Shutdown()
+{
+  WebCore::HRTFDatabaseLoader::shutdown();
+}
+
 }
 }
--- a/content/media/webaudio/WebAudioUtils.h
+++ b/content/media/webaudio/WebAudioUtils.h
@@ -205,15 +205,17 @@ struct WebAudioUtils {
       // If the floating point value is outside of the range of minimum
       // integral value for this type, just clamp to the minimum value.
       return numeric_limits<IntType>::min();
     }
 
     // Otherwise, this conversion must be well defined.
     return IntType(f);
   }
+
+  static void Shutdown();
 };
 
 }
 }
 
 #endif
 
--- a/content/media/webaudio/blink/HRTFDatabaseLoader.cpp
+++ b/content/media/webaudio/blink/HRTFDatabaseLoader.cpp
@@ -75,21 +75,23 @@ HRTFDatabaseLoader::HRTFDatabaseLoader(f
 
 HRTFDatabaseLoader::~HRTFDatabaseLoader()
 {
     MOZ_ASSERT(NS_IsMainThread());
 
     waitForLoaderThreadCompletion();
     m_hrtfDatabase.reset();
 
-    // Remove ourself from the map.
-    s_loaderMap->RemoveEntry(m_databaseSampleRate);
-    if (s_loaderMap->Count() == 0) {
-        delete s_loaderMap;
-        s_loaderMap = nullptr;
+    if (s_loaderMap) {
+        // Remove ourself from the map.
+        s_loaderMap->RemoveEntry(m_databaseSampleRate);
+        if (s_loaderMap->Count() == 0) {
+            delete s_loaderMap;
+            s_loaderMap = nullptr;
+        }
     }
 }
 
 class HRTFDatabaseLoader::ProxyReleaseEvent MOZ_FINAL : public nsRunnable {
 public:
     explicit ProxyReleaseEvent(HRTFDatabaseLoader* loader) : mLoader(loader) {}
     NS_IMETHOD Run() MOZ_OVERRIDE
     {
@@ -137,35 +139,41 @@ static void databaseLoaderEntry(void* th
     HRTFDatabaseLoader* loader = reinterpret_cast<HRTFDatabaseLoader*>(threadData);
     MOZ_ASSERT(loader);
     loader->load();
 }
 
 void HRTFDatabaseLoader::load()
 {
     MOZ_ASSERT(!NS_IsMainThread());
-    if (!m_hrtfDatabase.get()) {
-        // Load the default HRTF database.
-        m_hrtfDatabase = HRTFDatabase::create(m_databaseSampleRate);
-    }
+    MOZ_ASSERT(!m_hrtfDatabase.get(), "Called twice");
+    // Load the default HRTF database.
+    m_hrtfDatabase = HRTFDatabase::create(m_databaseSampleRate);
+    // Notifies the main thread of completion.  See loadAsynchronously().
+    Release();
 }
 
 void HRTFDatabaseLoader::loadAsynchronously()
 {
     MOZ_ASSERT(NS_IsMainThread());
+    MOZ_ASSERT(m_refCnt, "Must not be called before a reference is added");
+
+    // Add a reference so that the destructor won't run and wait for the
+    // loader thread, until load() has completed.
+    AddRef();
 
     MutexAutoLock locker(m_threadLock);
     
-    if (!m_hrtfDatabase.get() && !m_databaseLoaderThread) {
-        // Start the asynchronous database loading process.
-        m_databaseLoaderThread =
-            PR_CreateThread(PR_USER_THREAD, databaseLoaderEntry, this,
-                            PR_PRIORITY_NORMAL, PR_GLOBAL_THREAD,
-                            PR_JOINABLE_THREAD, 0);
-    }
+    MOZ_ASSERT(!m_hrtfDatabase.get() && !m_databaseLoaderThread,
+               "Called twice");
+    // Start the asynchronous database loading process.
+    m_databaseLoaderThread =
+        PR_CreateThread(PR_USER_THREAD, databaseLoaderEntry, this,
+                        PR_PRIORITY_NORMAL, PR_GLOBAL_THREAD,
+                        PR_JOINABLE_THREAD, 0);
 }
 
 bool HRTFDatabaseLoader::isLoaded() const
 {
     return m_hrtfDatabase.get();
 }
 
 void HRTFDatabaseLoader::waitForLoaderThreadCompletion()
@@ -175,9 +183,29 @@ void HRTFDatabaseLoader::waitForLoaderTh
     // waitForThreadCompletion() should not be called twice for the same thread.
     if (m_databaseLoaderThread) {
         DebugOnly<PRStatus> status = PR_JoinThread(m_databaseLoaderThread);
         MOZ_ASSERT(status == PR_SUCCESS, "PR_JoinThread failed");
     }
     m_databaseLoaderThread = nullptr;
 }
 
+PLDHashOperator
+HRTFDatabaseLoader::shutdownEnumFunc(LoaderByRateEntry *entry, void* unused)
+{
+    // Ensure the loader thread's reference is removed for leak analysis.
+    entry->mLoader->waitForLoaderThreadCompletion();
+    return PLDHashOperator::PL_DHASH_NEXT;
+}
+
+void HRTFDatabaseLoader::shutdown()
+{
+    MOZ_ASSERT(NS_IsMainThread());
+    if (s_loaderMap) {
+        // Set s_loaderMap to NULL so that the hashtable is not modified on
+        // reference release during enumeration.
+        nsTHashtable<LoaderByRateEntry>* loaderMap = s_loaderMap;
+        s_loaderMap = nullptr;
+        loaderMap->EnumerateEntries(shutdownEnumFunc, nullptr);
+        delete loaderMap;
+    }
+}
 } // namespace WebCore
--- a/content/media/webaudio/blink/HRTFDatabaseLoader.h
+++ b/content/media/webaudio/blink/HRTFDatabaseLoader.h
@@ -80,16 +80,18 @@ public:
 
     // waitForLoaderThreadCompletion() may be called more than once,
     // on any thread except m_databaseLoaderThread.
     void waitForLoaderThreadCompletion();
     
     HRTFDatabase* database() { return m_hrtfDatabase.get(); }
 
     float databaseSampleRate() const { return m_databaseSampleRate; }
+
+    static void shutdown();
     
     // Called in asynchronous loading thread.
     void load();
 
 private:
     // Both constructor and destructor must be called from the main thread.
     explicit HRTFDatabaseLoader(float sampleRate);
     ~HRTFDatabaseLoader();
@@ -107,16 +109,20 @@ private:
     public:
         LoaderByRateEntry(KeyTypePointer aKey)
             : nsFloatHashKey(aKey)
             , mLoader() // so PutEntry() will zero-initialize
         {
         }
         HRTFDatabaseLoader* mLoader;
     };
+
+    static PLDHashOperator shutdownEnumFunc(LoaderByRateEntry *entry,
+                                            void* unused);
+
     // Keeps track of loaders on a per-sample-rate basis.
     static nsTHashtable<LoaderByRateEntry> *s_loaderMap; // singleton
 
     mozilla::Atomic<int> m_refCnt;
 
     nsAutoRef<HRTFDatabase> m_hrtfDatabase;
 
     // Holding a m_threadLock is required when accessing m_databaseLoaderThread.
--- a/layout/build/nsLayoutStatics.cpp
+++ b/layout/build/nsLayoutStatics.cpp
@@ -87,16 +87,17 @@
 #include "WMFDecoder.h"
 #endif
 
 #ifdef MOZ_GSTREAMER
 #include "GStreamerFormatHelper.h"
 #endif
 
 #include "AudioStream.h"
+#include "WebAudioUtils.h"
 
 #ifdef MOZ_WIDGET_GONK
 #include "nsVolumeService.h"
 using namespace mozilla::system;
 #endif
 
 #include "nsError.h"
 
@@ -348,16 +349,17 @@ nsLayoutStatics::Shutdown()
   MediaPluginHost::Shutdown();
 #endif
 
 #ifdef MOZ_GSTREAMER
   GStreamerFormatHelper::Shutdown();
 #endif
 
   AudioStream::ShutdownLibrary();
+  WebAudioUtils::Shutdown();
 
 #ifdef MOZ_WMF
   WMFDecoder::UnloadDLLs();
 #endif
 
 #ifdef MOZ_WIDGET_GONK
   nsVolumeService::Shutdown();
 #endif