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
--- 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 */