Bug 1476250 - Simplify HAL initialization and shutdown to reduce the …

…chance of leaks and UAFs r=froydnj

This patch initializes some HAL components greedily so that we can get rid of
lazy initializers within the code. Observers are still lazily initialized
because they can be instanced within content processes but that doesn't always
happen and we don't want to pay the memory price for structures we don't use.

Shutdown is now happening at a fixed time for all HAL components save
WakeLocks. This ensures that we don't destroy an object while still iterating
over it, something that could happen before.

Finally a workaround for a compiler limitation has been removed.

Differential Revision: https://phabricator.services.mozilla.com/D3100

--HG--
extra : moz-landing-system : lando

hal/Hal.h

@@ -45,6 +45,17 @@ class WindowIdentifier;
 
 namespace MOZ_HAL_NAMESPACE {
 
+/**
+ * Initializes the HAL. This must be called before any other HAL function.
+ */
+void Init();
+
+/**
+ * Shuts down the HAL. Besides freeing all the used resources this will check
+ * that all observers have been properly deregistered and assert if not.
+ */
+void Shutdown();
+
 /**
  * Turn the default vibrator device on/off per the pattern specified
  * by |pattern|.  Each element in the pattern is the number of

hal/HalWakeLock.cpp

@@ -36,23 +36,15 @@ typedef nsClassHashtable<nsStringHashKey, ProcessLockTable> LockTable;
 
 int sActiveListeners = 0;
 StaticAutoPtr<LockTable> sLockTable;
-bool sInitialized = false;
 bool sIsShuttingDown = false;
 
 WakeLockInformation
 WakeLockInfoFromLockCount(const nsAString& aTopic, const LockCount& aLockCount)
 {
-  // TODO: Once we abandon b2g18, we can switch this to use the
-  // WakeLockInformation constructor, which is better because it doesn't let us
-  // forget to assign a param.  For now we have to do it this way, because
-  // b2g18 doesn't have the nsTArray <--> InfallibleTArray conversion (bug
-  // 819791).
-
-  WakeLockInformation info;
-  info.topic() = aTopic;
-  info.numLocks() = aLockCount.numLocks;
-  info.numHidden() = aLockCount.numHidden;
-  info.lockingProcesses().AppendElements(aLockCount.processes);
+  nsString topic(aTopic);
+  WakeLockInformation info(topic, aLockCount.numLocks, aLockCount.numHidden,
+                           aLockCount.processes);
+
   return info;
 }
 
@@ -146,11 +138,16 @@ CleanupOnContentShutdown::Observe(nsISupports* aSubject, const char* aTopic, con
   return NS_OK;
 }
 
+} // namespace
+
+namespace mozilla {
+
+namespace hal {
+
 void
-Init()
+WakeLockInit()
 {
   sLockTable = new LockTable();
-  sInitialized = true;
 
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
@@ -159,11 +156,6 @@ Init()
   }
 }
 
-} // namespace
-
-namespace mozilla {
-
-namespace hal {
 
 WakeLockState
 ComputeWakeLockState(int aNumLocks, int aNumHidden)
@@ -205,9 +197,6 @@ ModifyWakeLock(const nsAString& aTopic,
   if (sIsShuttingDown) {
     return;
   }
-  if (!sInitialized) {
-    Init();
-  }
 
   ProcessLockTable* table = sLockTable->Get(aTopic);
   LockCount processCount;
@@ -264,9 +253,6 @@ GetWakeLockInfo(const nsAString& aTopic, WakeLockInformation* aWakeLockInfo)
     *aWakeLockInfo = WakeLockInformation();
     return;
   }
-  if (!sInitialized) {
-    Init();
-  }
 
   ProcessLockTable* table = sLockTable->Get(aTopic);
   if (!table) {

hal/HalWakeLockInternal.h

@@ -0,0 +1,17 @@
+/* -*- Mode: C++; tab-width: 40; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* 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/. */
+
+#ifndef __HAL_WAKELOCK_INTERNAL_H_
+#define __HAL_WAKELOCK_INTERNAL_H_
+
+namespace mozilla {
+namespace hal {
+
+void WakeLockInit();
+
+} // namespace hal
+} // namespace mozilla
+
+#endif /* __HAL_WAKELOCK_INTERNAL_H_ */

widget/android/nsAppShell.cpp

@@ -405,6 +405,8 @@ nsAppShell::nsAppShell()
         sAppShell = this;
     }
 
+    hal::Init();
+
     if (!XRE_IsParentProcess()) {
         if (jni::IsAvailable()) {
             GeckoThreadSupport::Init();
@@ -473,6 +475,8 @@ nsAppShell::~nsAppShell()
         sWakeLockListener = nullptr;
     }
 
+    hal::Shutdown();
+
     if (jni::IsAvailable() && XRE_IsParentProcess()) {
         DestroyAndroidUiThread();
         AndroidBridge::DeconstructBridge();

widget/android/nsAppShell.h

@@ -257,6 +257,8 @@ class nsAppShell :
 
     } mEventQueue;
 
+private:
+
     mozilla::CondVar mSyncRunFinished;
     bool mSyncRunQuit;
 

widget/cocoa/nsAppShell.mm

@@ -33,6 +33,7 @@
 #include "mozilla/BackgroundHangMonitor.h"
 #include "GeckoProfiler.h"
 #include "ScreenHelperCocoa.h"
+#include "mozilla/Hal.h"
 #include "mozilla/widget/ScreenManager.h"
 #include "HeadlessScreenHelper.h"
 #include "pratom.h"
@@ -237,6 +238,8 @@ - (void)beginMenuTracking:(NSNotification*)aNotification;
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
+  hal::Shutdown();
+
   if (mCFRunLoop) {
     if (mCFRunLoopSource) {
       ::CFRunLoopRemoveSource(mCFRunLoop, mCFRunLoopSource,
@@ -356,6 +359,8 @@ - (void)beginMenuTracking:(NSNotification*)aNotification;
 
   ::CFRunLoopAddSource(mCFRunLoop, mCFRunLoopSource, kCFRunLoopCommonModes);
 
+  hal::Init();
+
   if (XRE_IsParentProcess()) {
     ScreenManager& screenManager = ScreenManager::GetSingleton();
 

widget/gtk/nsAppShell.cpp

@@ -15,6 +15,7 @@
 #include "mozilla/Logging.h"
 #include "prenv.h"
 #include "mozilla/BackgroundHangMonitor.h"
+#include "mozilla/Hal.h"
 #include "mozilla/Unused.h"
 #include "mozilla/WidgetUtils.h"
 #include "GeckoProfiler.h"
@@ -134,6 +135,8 @@ nsAppShell::EventProcessorCallback(GIOChannel *source,
 
 nsAppShell::~nsAppShell()
 {
+    mozilla::hal::Shutdown();
+
     if (mTag)
         g_source_remove(mTag);
     if (mPipeFDs[0])
@@ -151,6 +154,8 @@ nsAppShell::Init()
     // is a no-op.
     g_type_init();
 
+    mozilla::hal::Init();
+
 #ifdef MOZ_ENABLE_DBUS
     if (XRE_IsParentProcess()) {
         nsCOMPtr<nsIPowerManagerService> powerManagerService =

widget/windows/nsAppShell.cpp

@@ -16,6 +16,7 @@
 #include "WinIMEHandler.h"
 #include "mozilla/widget/AudioSession.h"
 #include "mozilla/BackgroundHangMonitor.h"
+#include "mozilla/Hal.h"
 #include "nsIDOMWakeLockListener.h"
 #include "nsIPowerManagerService.h"
 #include "mozilla/StaticPtr.h"
@@ -210,6 +211,8 @@ nsAppShell::EventWindowProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
 
 nsAppShell::~nsAppShell()
 {
+  hal::Shutdown();
+
   if (mEventWnd) {
     // DestroyWindow doesn't do anything when called from a non UI thread.
     // Since mEventWnd was created on the UI thread, it must be destroyed on
@@ -327,6 +330,8 @@ nsAppShell::Init()
 {
   LSPAnnotate();
 
+  hal::Init();
+
   mozilla::ipc::windows::InitUIThread();
 
   sTaskbarButtonCreatedMsg = ::RegisterWindowMessageW(kTaskbarButtonEventId);