Bug 1308418: [cubeb] P1. Fix mutex construction. r?padenot draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Tue, 18 Oct 2016 16:40:36 +1100
changeset 426227 67e81c8bfcc888415b3b5694d7eda862139c9fb9
parent 426151 56b3f2c6f53e72698fea6c25130efceef2a26548
child 426228 b3378ab03bd1e96499372e483d238e3dd2b14bee
push id32659
push userbmo:jyavenard@mozilla.com
push dateTue, 18 Oct 2016 06:12:24 +0000
reviewerspadenot
bugs1308418
milestone52.0a1
Bug 1308418: [cubeb] P1. Fix mutex construction. r?padenot We delete owned_critical_section move constructor on unix as it should never be used ever. On windows, to minimize the change, we implement the operator, but it's less than ideal to construct an object using calloc. MozReview-Commit-ID: T0WUHBUxka
media/libcubeb/src/cubeb_audiounit.cpp
media/libcubeb/src/cubeb_utils_unix.h
media/libcubeb/src/cubeb_utils_win.h
--- a/media/libcubeb/src/cubeb_audiounit.cpp
+++ b/media/libcubeb/src/cubeb_audiounit.cpp
@@ -76,16 +76,27 @@ typedef UInt32  AudioFormatFlags;
  * low, but this does not work in practice. Lie and say the minimum is 256
  * frames. */
 const uint32_t SAFE_MIN_LATENCY_FRAMES = 256;
 const uint32_t SAFE_MAX_LATENCY_FRAMES = 512;
 
 extern cubeb_ops const audiounit_ops;
 
 struct cubeb {
+  cubeb()
+  : ops(0)
+  , active_streams(0)
+  , limit_streams(0)
+  , collection_changed_callback(0)
+  , collection_changed_user_ptr(0)
+  , collection_changed_devtype(CUBEB_DEVICE_TYPE_UNKNOWN)
+  , devtype_device_count(0)
+  , devtype_device_array(0)
+  { }
+
   cubeb_ops const * ops;
   owned_critical_section mutex;
   int active_streams;
   int limit_streams;
   cubeb_device_collection_changed_callback collection_changed_callback;
   void * collection_changed_user_ptr;
   /* Differentiate input from output devices. */
   cubeb_device_type collection_changed_devtype;
@@ -148,16 +159,40 @@ public:
   }
 
 private:
   auto_array<float> * float_ar;
   auto_array<short> * short_ar;
 };
 
 struct cubeb_stream {
+  cubeb_stream()
+  : context(0)
+  , data_callback(0)
+  , state_callback(0)
+  , device_changed_callback(0)
+  , user_ptr(0)
+  , input_hw_rate(0)
+  , input_linear_buffer(0)
+  , input_buffer_frames(0)
+  , frames_played(0)
+  , frames_queued(0)
+  , frames_read(0)
+  , shutdown(0)
+  , draining(0)
+  , current_latency_frames(0)
+  , hw_latency_frames(0)
+  , panning(0)
+  , resampler(0)
+  {
+    PodZero(&input_desc, 1);
+    PodZero(&output_desc, 1);
+    PodZero(&input_unit, 1);
+    PodZero(&output_unit, 1);
+  }
   cubeb * context;
   cubeb_data_callback data_callback;
   cubeb_state_callback state_callback;
   cubeb_device_changed_callback device_changed_callback;
   /* User pointer of data_callback */
   void * user_ptr;
   /* Format descriptions */
   AudioStreamBasicDescription input_desc;
@@ -434,22 +469,19 @@ int
 audiounit_init(cubeb ** context, char const * context_name)
 {
   cubeb * ctx;
 
   *context = NULL;
 
   ctx = new cubeb;
   assert(ctx);
-  PodZero(ctx, 1);
 
   ctx->ops = &audiounit_ops;
 
-  ctx->mutex = owned_critical_section();
-
   ctx->active_streams = 0;
 
   ctx->limit_streams = kCFCoreFoundationVersionNumber < kCFCoreFoundationVersionNumber10_7;
 #if !TARGET_OS_IPHONE
   cubeb_set_coreaudio_notification_runloop();
 #endif
 
   *context = ctx;
@@ -1128,28 +1160,26 @@ audiounit_stream_init(cubeb * context,
     if (r != CUBEB_OK) {
       LOG("AudioUnit creation for output failed.");
       return r;
     }
   }
 
   stm = new cubeb_stream;
   assert(stm);
-  PodZero(stm, 1);
 
   /* These could be different in the future if we have both
    * full-duplex stream and different devices for input vs output. */
   stm->input_unit  = (input_stream_params != NULL) ? input_unit : NULL;
   stm->output_unit = (output_stream_params != NULL) ? output_unit : NULL;
   stm->context = context;
   stm->data_callback = data_callback;
   stm->state_callback = state_callback;
   stm->user_ptr = user_ptr;
   stm->device_changed_callback = NULL;
-  stm->mutex = owned_critical_section();
 
   /* Init data members where necessary */
   stm->hw_latency_frames = UINT64_MAX;
 
   /* Silently clamp the latency down to the platform default, because we
    * synthetize the clock from the callbacks, and we want the clock to update
    * often. */
   latency_frames = audiounit_clamp_latency(stm, latency_frames);
--- a/media/libcubeb/src/cubeb_utils_unix.h
+++ b/media/libcubeb/src/cubeb_utils_unix.h
@@ -43,16 +43,18 @@ public:
     int r =
 #endif
     pthread_mutex_destroy(&mutex);
 #ifdef DEBUG
     assert(r == 0);
 #endif
   }
 
+  owned_critical_section& operator=(owned_critical_section&& other) = delete;
+
   void enter()
   {
 #ifdef DEBUG
     int r =
 #endif
     pthread_mutex_lock(&mutex);
 #ifdef DEBUG
     assert(r == 0 && "Deadlock");
--- a/media/libcubeb/src/cubeb_utils_win.h
+++ b/media/libcubeb/src/cubeb_utils_win.h
@@ -17,21 +17,35 @@ class owned_critical_section
 {
 public:
   owned_critical_section()
 #ifdef DEBUG
     : owner(0)
 #endif
   {
     InitializeCriticalSection(&critical_section);
+    owning = true;
   }
 
   ~owned_critical_section()
   {
-    DeleteCriticalSection(&critical_section);
+    if (owning) {
+      DeleteCriticalSection(&critical_section);
+      owning = false;
+    }
+  }
+
+  owned_critical_section& operator=(owned_critical_section&& other)
+  {
+    if (owning) {
+      DeleteCriticalSection(&critical_section);
+    }
+    critical_section = other.critical_section;
+    other.owning = false;
+    return *this;
   }
 
   void enter()
   {
     EnterCriticalSection(&critical_section);
 #ifdef DEBUG
     XASSERT(owner != GetCurrentThreadId() && "recursive locking");
     owner = GetCurrentThreadId();
@@ -53,15 +67,16 @@ public:
   {
 #ifdef DEBUG
     /* This implies owner != 0, because GetCurrentThreadId cannot return 0. */
     XASSERT(owner == GetCurrentThreadId());
 #endif
   }
 
 private:
+  bool owning;
   CRITICAL_SECTION critical_section;
 #ifdef DEBUG
   DWORD owner;
 #endif
 };
 
 #endif /* CUBEB_UTILS_WIN */