Bug 1471164 - Beta uplift for cubeb crash in stream destroy. r=padenot, a=lizzard
authorAlex Chronopoulos <achronop@gmail.com>
Wed, 11 Jul 2018 20:31:46 -0400
changeset 477907 77d51fa00f585ca0682750cf934c1ece93b8ae83
parent 477906 799e2bc3b592965fe806bcb740d48615db15cc36
child 477908 2b3b18d4f22d8e27de3d83eaed634039e9be37fa
push id9462
push userryanvm@gmail.com
push dateThu, 12 Jul 2018 00:35:00 +0000
treeherdermozilla-beta@bf0bb0c06878 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot, lizzard
bugs1471164
milestone62.0
Bug 1471164 - Beta uplift for cubeb crash in stream destroy. r=padenot, a=lizzard
media/libcubeb/audiounit-stream-destroy-crash.patch
media/libcubeb/src/cubeb_audiounit.cpp
media/libcubeb/update.sh
new file mode 100644
--- /dev/null
+++ b/media/libcubeb/audiounit-stream-destroy-crash.patch
@@ -0,0 +1,98 @@
+commit 27be938448d0345087719d764fecc57692046a8b
+Author: Alex Chronopoulos <achronop@gmail.com>
+Date:   Mon Jun 25 23:37:59 2018 +0200
+
+    audiounit: new method on failed stream init to avoid deadlock (BMO 1470113). (#445)
+    
+    * audiounit: new method on failed stream init to avoid deadlock (BMO 1470113).
+    * use an internal_destroy method to avoid duplication
+
+diff --git a/src/cubeb_audiounit.cpp b/src/cubeb_audiounit.cpp
+index a9ea6a2..6163ed7 100644
+--- a/src/cubeb_audiounit.cpp
++++ b/src/cubeb_audiounit.cpp
+@@ -2585,6 +2585,8 @@ cubeb_stream::cubeb_stream(cubeb * context)
+   PodZero(&output_desc, 1);
+ }
+ 
++static void audiounit_stream_destroy_internal(cubeb_stream * stm);
++
+ static int
+ audiounit_stream_init(cubeb * context,
+                       cubeb_stream ** stream,
+@@ -2602,14 +2604,10 @@ audiounit_stream_init(cubeb * context,
+   auto_lock context_lock(context->mutex);
+   audiounit_increment_active_streams(context);
+   unique_ptr<cubeb_stream, decltype(&audiounit_stream_destroy)> stm(new cubeb_stream(context),
+-                                                                    audiounit_stream_destroy);
++                                                                    audiounit_stream_destroy_internal);
+   int r;
+   *stream = NULL;
+   assert(latency_frames > 0);
+-  if ((input_device && !input_stream_params) ||
+-      (output_device && !output_stream_params)) {
+-    return CUBEB_ERROR_INVALID_PARAMETER;
+-  }
+ 
+   /* These could be different in the future if we have both
+    * full-duplex stream and different devices for input vs output. */
+@@ -2617,6 +2615,11 @@ audiounit_stream_init(cubeb * context,
+   stm->state_callback = state_callback;
+   stm->user_ptr = user_ptr;
+   stm->latency_frames = latency_frames;
++
++  if ((input_device && !input_stream_params) ||
++      (output_device && !output_stream_params)) {
++    return CUBEB_ERROR_INVALID_PARAMETER;
++  }
+   if (input_stream_params) {
+     stm->input_stream_params = *input_stream_params;
+     r = audiounit_set_device_info(stm.get(), reinterpret_cast<uintptr_t>(input_device), INPUT);
+@@ -2687,33 +2690,39 @@ audiounit_close_stream(cubeb_stream *stm)
+ }
+ 
+ static void
+-audiounit_stream_destroy(cubeb_stream * stm)
++audiounit_stream_destroy_internal(cubeb_stream *stm)
+ {
+-  stm->shutdown = true;
++  stm->context->mutex.assert_current_thread_owns();
+ 
+   int r = audiounit_uninstall_system_changed_callback(stm);
+   if (r != CUBEB_OK) {
+     LOG("(%p) Could not uninstall the device changed callback", stm);
+   }
+-
+   r = audiounit_uninstall_device_changed_callback(stm);
+   if (r != CUBEB_OK) {
+     LOG("(%p) Could not uninstall all device change listeners", stm);
+   }
+ 
+-  {
++  auto_lock lock(stm->mutex);
++  audiounit_close_stream(stm);
++  assert(audiounit_active_streams(stm->context) >= 1);
++  audiounit_decrement_active_streams(stm->context);
++}
++
++static void
++audiounit_stream_destroy(cubeb_stream * stm)
++{
++  if (!stm->shutdown.load()){
+     auto_lock context_lock(stm->context->mutex);
+     audiounit_stream_stop_internal(stm);
++    stm->shutdown = true;
+   }
+ 
+   // Execute close in serial queue to avoid collision
+   // with reinit when un/plug devices
+   dispatch_sync(stm->context->serial_queue, ^() {
+-    auto_lock lock(stm->mutex);
+-    audiounit_close_stream(stm);
+     auto_lock context_lock(stm->context->mutex);
+-    assert(audiounit_active_streams(stm->context) >= 1);
+-    audiounit_decrement_active_streams(stm->context);
++    audiounit_stream_destroy_internal(stm);
+   });
+ 
+   LOG("Cubeb stream (%p) destroyed successful.", stm);
--- a/media/libcubeb/src/cubeb_audiounit.cpp
+++ b/media/libcubeb/src/cubeb_audiounit.cpp
@@ -2580,16 +2580,18 @@ cubeb_stream::cubeb_stream(cubeb * conte
   : context(context)
   , resampler(nullptr, cubeb_resampler_destroy)
   , mixer(nullptr, cubeb_mixer_destroy)
 {
   PodZero(&input_desc, 1);
   PodZero(&output_desc, 1);
 }
 
+static void audiounit_stream_destroy_internal(cubeb_stream * stm);
+
 static int
 audiounit_stream_init(cubeb * context,
                       cubeb_stream ** stream,
                       char const * /* stream_name */,
                       cubeb_devid input_device,
                       cubeb_stream_params * input_stream_params,
                       cubeb_devid output_device,
                       cubeb_stream_params * output_stream_params,
@@ -2597,31 +2599,32 @@ audiounit_stream_init(cubeb * context,
                       cubeb_data_callback data_callback,
                       cubeb_state_callback state_callback,
                       void * user_ptr)
 {
   assert(context);
   auto_lock context_lock(context->mutex);
   audiounit_increment_active_streams(context);
   unique_ptr<cubeb_stream, decltype(&audiounit_stream_destroy)> stm(new cubeb_stream(context),
-                                                                    audiounit_stream_destroy);
+                                                                    audiounit_stream_destroy_internal);
   int r;
   *stream = NULL;
   assert(latency_frames > 0);
-  if ((input_device && !input_stream_params) ||
-      (output_device && !output_stream_params)) {
-    return CUBEB_ERROR_INVALID_PARAMETER;
-  }
 
   /* These could be different in the future if we have both
    * full-duplex stream and different devices for input vs output. */
   stm->data_callback = data_callback;
   stm->state_callback = state_callback;
   stm->user_ptr = user_ptr;
   stm->latency_frames = latency_frames;
+
+  if ((input_device && !input_stream_params) ||
+      (output_device && !output_stream_params)) {
+    return CUBEB_ERROR_INVALID_PARAMETER;
+  }
   if (input_stream_params) {
     stm->input_stream_params = *input_stream_params;
     r = audiounit_set_device_info(stm.get(), reinterpret_cast<uintptr_t>(input_device), INPUT);
     if (r != CUBEB_OK) {
       LOG("(%p) Fail to set device info for input.", stm.get());
       return r;
     }
   }
@@ -2682,43 +2685,49 @@ audiounit_close_stream(cubeb_stream *stm
 
   if (stm->aggregate_device_id) {
     audiounit_destroy_aggregate_device(stm->plugin_id, &stm->aggregate_device_id);
     stm->aggregate_device_id = 0;
   }
 }
 
 static void
-audiounit_stream_destroy(cubeb_stream * stm)
+audiounit_stream_destroy_internal(cubeb_stream *stm)
 {
-  stm->shutdown = true;
+  stm->context->mutex.assert_current_thread_owns();
 
   int r = audiounit_uninstall_system_changed_callback(stm);
   if (r != CUBEB_OK) {
     LOG("(%p) Could not uninstall the device changed callback", stm);
   }
-
   r = audiounit_uninstall_device_changed_callback(stm);
   if (r != CUBEB_OK) {
     LOG("(%p) Could not uninstall all device change listeners", stm);
   }
 
-  {
+  auto_lock lock(stm->mutex);
+  audiounit_close_stream(stm);
+  assert(audiounit_active_streams(stm->context) >= 1);
+  audiounit_decrement_active_streams(stm->context);
+}
+
+static void
+audiounit_stream_destroy(cubeb_stream * stm)
+{
+  if (!stm->shutdown.load()){
     auto_lock context_lock(stm->context->mutex);
     audiounit_stream_stop_internal(stm);
+    stm->shutdown = true;
   }
 
   // Execute close in serial queue to avoid collision
   // with reinit when un/plug devices
   dispatch_sync(stm->context->serial_queue, ^() {
-    auto_lock lock(stm->mutex);
-    audiounit_close_stream(stm);
     auto_lock context_lock(stm->context->mutex);
-    assert(audiounit_active_streams(stm->context) >= 1);
-    audiounit_decrement_active_streams(stm->context);
+    audiounit_stream_destroy_internal(stm);
   });
 
   LOG("Cubeb stream (%p) destroyed successful.", stm);
   delete stm;
 }
 
 void
 audiounit_stream_start_internal(cubeb_stream * stm)
--- a/media/libcubeb/update.sh
+++ b/media/libcubeb/update.sh
@@ -78,8 +78,10 @@ echo "Applying disable-assert.patch on t
 patch -p3 < disable-assert.patch
 
 echo "Applying prefer-pulse-rust.patch on top of $rev"
 patch -p3 < prefer-pulse-rust.patch
 
 echo "Applying disable-device-switching.patch on top of $rev"
 patch -p3 < disable-device-switching.patch
 
+echo "Apply audiounit-stream-destroy-crash.patch on top of $rev"
+patch -p1 < audiounit-stream-destroy-crash.patch