Backed out changeset e98549c5dd9d (bug 1347519) for RunWatchdog shutdown hangs in Mac debug mochitest-1
authorPhil Ringnalda <philringnalda@gmail.com>
Tue, 05 Sep 2017 21:39:55 -0700
changeset 428595 867b17852af860ea301a31406c88dc84b8aa4a88
parent 428594 9b75676259d8b054e5edd36c311305ad9ae61536
child 428596 ef6cf161ece380d18d976c61d534ded83053bfed
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)
bugs1347519
milestone57.0a1
backs oute98549c5dd9dd6fd6582ed74e46152ce3bf56a0f
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
Backed out changeset e98549c5dd9d (bug 1347519) for RunWatchdog shutdown hangs in Mac debug mochitest-1 MozReview-Commit-ID: Ew32xfcEJlx
dom/gamepad/cocoa/CocoaGamepad.cpp
--- a/dom/gamepad/cocoa/CocoaGamepad.cpp
+++ b/dom/gamepad/cocoa/CocoaGamepad.cpp
@@ -18,19 +18,16 @@
 #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;
 
@@ -201,40 +198,36 @@ 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
@@ -246,51 +239,16 @@ 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;
   }
@@ -469,29 +427,16 @@ 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);
@@ -519,18 +464,16 @@ 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;
 
@@ -587,49 +530,48 @@ 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()
 {
-  CFRunLoopSourceSignal(mShutdownMsg);
+  IOHIDManagerRef manager = (IOHIDManagerRef)mManager;
+  CFRunLoopStop(mMonitorRunLoop);
+  if (manager) {
+    IOHIDManagerClose(manager, 0);
+    CFRelease(manager);
+    mManager = nullptr;
+  }
+  mMonitorThread->Shutdown();
 }
 
 } // namespace
 
 namespace mozilla {
 namespace dom {
 
+DarwinGamepadService* gService = nullptr;
+
 void StartGamepadMonitoring()
 {
   if (gService) {
     return;
   }
 
   gService = new DarwinGamepadService();
   gService->Startup();
@@ -637,12 +579,14 @@ void StartGamepadMonitoring()
 
 void StopGamepadMonitoring()
 {
   if (!gService) {
     return;
   }
 
   gService->Shutdown();
+  delete gService;
+  gService = nullptr;
 }
 
 } // namespace dom
 } // namespace mozilla