Bug 1347519 - Use CFRunLoopSource for passing shutdown signal r=qdot
☠☠ backed out by 867b17852af8 ☠ ☠
authorChih-Yi Leu <cleu@mozilla.com>
Fri, 01 Sep 2017 14:41:35 +0800
changeset 428585 e98549c5dd9dd6fd6582ed74e46152ce3bf56a0f
parent 428584 66954c786c3a12ff827b2463b52319309a0949d3
child 428586 b502be79aeb198e3d701ad4f61836b08fd1452eb
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1347519
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1347519 - Use CFRunLoopSource for passing shutdown signal r=qdot MozReview-Commit-ID: 9TgZaNywIQC
dom/gamepad/cocoa/CocoaGamepad.cpp
--- a/dom/gamepad/cocoa/CocoaGamepad.cpp
+++ b/dom/gamepad/cocoa/CocoaGamepad.cpp
@@ -18,16 +18,19 @@
 #include <stdio.h>
 #include <vector>
 
 namespace {
 
 using namespace mozilla;
 using namespace mozilla::dom;
 using std::vector;
+class DarwinGamepadService;
+
+DarwinGamepadService* gService = nullptr;
 
 struct Button {
   int id;
   bool analog;
   IOHIDElementRef element;
   CFIndex min;
   CFIndex max;
 
@@ -198,36 +201,40 @@ void Gamepad::init(IOHIDDeviceRef device
 
 class DarwinGamepadService {
  private:
   IOHIDManagerRef mManager;
   vector<Gamepad> mGamepads;
 
   //Workaround to support running in background thread
   CFRunLoopRef mMonitorRunLoop;
+  CFRunLoopSourceRef mShutdownMsg;
   nsCOMPtr<nsIThread> mMonitorThread;
+  nsCOMPtr<nsIThread> mBackgroundThread;
 
   static void DeviceAddedCallback(void* data, IOReturn result,
                                   void* sender, IOHIDDeviceRef device);
   static void DeviceRemovedCallback(void* data, IOReturn result,
                                     void* sender, IOHIDDeviceRef device);
   static void InputValueChangedCallback(void* data, IOReturn result,
                                         void* sender, IOHIDValueRef newValue);
+  static void ReceiveShutdownMsgCallback(void* info);
 
   void DeviceAdded(IOHIDDeviceRef device);
   void DeviceRemoved(IOHIDDeviceRef device);
   void InputValueChanged(IOHIDValueRef value);
   void StartupInternal();
 
  public:
   DarwinGamepadService();
   ~DarwinGamepadService();
   void Startup();
   void Shutdown();
   friend class DarwinGamepadServiceStartupRunnable;
+  friend class DarwinGamepadServiceShutdownRunnable;
 };
 
 class DarwinGamepadServiceStartupRunnable final : public Runnable
 {
  private:
   ~DarwinGamepadServiceStartupRunnable() {}
   // This Runnable schedules startup of DarwinGamepadService
   // in a new thread, pointer to DarwinGamepadService is only
@@ -239,16 +246,51 @@ class DarwinGamepadServiceStartupRunnabl
   NS_IMETHOD Run() override
   {
     MOZ_ASSERT(mService);
     mService->StartupInternal();
     return NS_OK;
   }
 };
 
+class DarwinGamepadServiceShutdownRunnable final : public Runnable
+{
+private:
+  ~DarwinGamepadServiceShutdownRunnable() {}
+public:
+  // This Runnable schedules shutdown of DarwinGamepadService
+  // in background thread.
+  explicit DarwinGamepadServiceShutdownRunnable()
+    : Runnable("DarwinGamepadServiceStartupRunnable") {}
+  NS_IMETHOD Run() override
+  {
+    MOZ_ASSERT(gService);
+    MOZ_ASSERT(NS_GetCurrentThread() == gService->mBackgroundThread);
+
+    IOHIDManagerRef manager = (IOHIDManagerRef)gService->mManager;
+
+    if (gService->mShutdownMsg) {
+      CFRunLoopRemoveSource(gService->mMonitorRunLoop,
+                            gService->mShutdownMsg,
+                            kCFRunLoopCommonModes);
+      CFRelease(gService->mShutdownMsg);
+      gService->mShutdownMsg = nullptr;
+    }
+    if (manager) {
+      IOHIDManagerClose(manager, 0);
+      CFRelease(manager);
+      gService->mManager = nullptr;
+    }
+    gService->mMonitorThread->Shutdown();
+    delete gService;
+    gService = nullptr;
+    return NS_OK;
+  }
+};
+
 void
 DarwinGamepadService::DeviceAdded(IOHIDDeviceRef device)
 {
   RefPtr<GamepadPlatformService> service =
     GamepadPlatformService::GetParentService();
   if (!service) {
     return;
   }
@@ -427,16 +469,29 @@ DarwinGamepadService::InputValueChangedC
                                                IOReturn result,
                                                void* sender,
                                                IOHIDValueRef newValue)
 {
   DarwinGamepadService* service = (DarwinGamepadService*)data;
   service->InputValueChanged(newValue);
 }
 
+void
+DarwinGamepadService::ReceiveShutdownMsgCallback(void* info)
+{
+  // Calling CFRunLoopStop ouside the RunLoop thread may cause crashes
+  // inside libpthread, so we need to stop the RunLoop here and dispatch
+  // rest cleanup actions back to PBackground thread to make sure everything
+  // work correctly.
+  DarwinGamepadService* service = static_cast<DarwinGamepadService*>(info);
+  CFRunLoopStop(service->mMonitorRunLoop);
+  RefPtr<Runnable> shutdownTask =  new DarwinGamepadServiceShutdownRunnable();
+  service->mBackgroundThread->IdleDispatch(shutdownTask.forget());
+}
+
 static CFMutableDictionaryRef
 MatchingDictionary(UInt32 inUsagePage, UInt32 inUsage)
 {
   CFMutableDictionaryRef dict =
     CFDictionaryCreateMutable(kCFAllocatorDefault,
                               0,
                               &kCFTypeDictionaryKeyCallBacks,
                               &kCFTypeDictionaryValueCallBacks);
@@ -464,16 +519,18 @@ MatchingDictionary(UInt32 inUsagePage, U
 }
 
 DarwinGamepadService::DarwinGamepadService() : mManager(nullptr) {}
 
 DarwinGamepadService::~DarwinGamepadService()
 {
   if (mManager != nullptr)
     CFRelease(mManager);
+  mMonitorThread = nullptr;
+  mBackgroundThread = nullptr;
 }
 
 void
 DarwinGamepadService::StartupInternal()
 {
   if (mManager != nullptr)
     return;
 
@@ -530,48 +587,49 @@ DarwinGamepadService::StartupInternal()
 
   mManager = manager;
 
   // We held the handle of the CFRunLoop to make sure we
   // can shut it down explicitly by CFRunLoopStop in another
   // thread.
   mMonitorRunLoop = CFRunLoopGetCurrent();
 
+  CFRunLoopSourceContext context;
+  memset(&context, 0, sizeof(CFRunLoopSourceContext));
+  context.info = gService;
+  context.perform = ReceiveShutdownMsgCallback;
+
+  mShutdownMsg = CFRunLoopSourceCreate(kCFAllocatorDefault, 0,
+                                       &context);
+  CFRunLoopAddSource(mMonitorRunLoop, mShutdownMsg, kCFRunLoopDefaultMode);
+
   // CFRunLoopRun() is a blocking message loop when it's called in
   // non-main thread so this thread cannot receive any other runnables
   // and nsITimer timeout events after it's called.
   CFRunLoopRun();
 }
 
 void DarwinGamepadService::Startup()
 {
+  mBackgroundThread = NS_GetCurrentThread();
   Unused << NS_NewNamedThread("Gamepad",
                               getter_AddRefs(mMonitorThread),
                               new DarwinGamepadServiceStartupRunnable(this));
 }
 
 void DarwinGamepadService::Shutdown()
 {
-  IOHIDManagerRef manager = (IOHIDManagerRef)mManager;
-  CFRunLoopStop(mMonitorRunLoop);
-  if (manager) {
-    IOHIDManagerClose(manager, 0);
-    CFRelease(manager);
-    mManager = nullptr;
-  }
-  mMonitorThread->Shutdown();
+  CFRunLoopSourceSignal(mShutdownMsg);
 }
 
 } // namespace
 
 namespace mozilla {
 namespace dom {
 
-DarwinGamepadService* gService = nullptr;
-
 void StartGamepadMonitoring()
 {
   if (gService) {
     return;
   }
 
   gService = new DarwinGamepadService();
   gService->Startup();
@@ -579,14 +637,12 @@ void StartGamepadMonitoring()
 
 void StopGamepadMonitoring()
 {
   if (!gService) {
     return;
   }
 
   gService->Shutdown();
-  delete gService;
-  gService = nullptr;
 }
 
 } // namespace dom
 } // namespace mozilla