Skip to content

Commit

Permalink
Bug 1619005 - Update cubeb-coreaudio to 799518a. r=padenot
Browse files Browse the repository at this point in the history
Pick commits:
799518a - Rework threading model (#55)
6e3e8e8 - Fix memory leak (#54)
996fcfa - Revise messages in test_switch_device
6fac4a6 - Isolate device tests (#53)
b78e817 - Save the duplicate tests and compilings when running sanitizers (#52)

Differential Revision: https://phabricator.services.mozilla.com/D67536

--HG--
rename : third_party/rust/cubeb-coreaudio/run_tests.sh => third_party/rust/cubeb-coreaudio/run_device_tests.sh
extra : moz-landing-system : lando
  • Loading branch information
ChunMinChang committed Mar 20, 2020
1 parent 773f537 commit 4c786ba
Show file tree
Hide file tree
Showing 15 changed files with 416 additions and 263 deletions.
2 changes: 1 addition & 1 deletion .cargo/config.in
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ rev = "5e870faf6f95d79d11efc813e56370ad124bbed5"
[source."https://github.com/ChunMinChang/cubeb-coreaudio-rs"]
git = "https://github.com/ChunMinChang/cubeb-coreaudio-rs"
replace-with = "vendored-sources"
rev = "4acd80233efa645ac79769f37b07d495c1b42070"
rev = "799518a033a0c780cfdb82a4eaabfd9681cb7f3b"

[source.crates-io]
replace-with = "vendored-sources"
Expand Down
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion third_party/rust/coreaudio-sys-utils/.cargo-checksum.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"files":{"Cargo.toml":"35acbd2f8633a6109f3d3e554bef8d847c049ce6ef7a5f570468819e41344d7f","src/aggregate_device.rs":"7d2bd5f5fd7f3d008ebb69ad81f522ca0cb73db6d7b3e50ed1a63ea26ff721f4","src/audio_object.rs":"df10160d9fd83a2c23a49e69b78d39db3a9d6389607df6acfc05821293b6af5f","src/audio_unit.rs":"d783878930df4923b57ad230138c0f3fd6b0b9bb80a39725092ff4c6615162d8","src/cf_mutable_dict.rs":"fc42edd270c6dfb02f123214d2d8e487bbd62b5bd923b71eec13190fd0104d2a","src/dispatch.rs":"195ca94cbc61948637bfdcbe22070a1e6d41e97cec22301df4e45dcef7b1c208","src/lib.rs":"bcc559d69ef6ed0cbea5b2a36fec89d8c011eb9da70e2f26c00f881ad97a2546","src/string.rs":"28f88b816c768bcfcc674a60d962b93f1c94e5e0f4cc8ed2a1301138b91039e7"},"package":null}
{"files":{"Cargo.toml":"35acbd2f8633a6109f3d3e554bef8d847c049ce6ef7a5f570468819e41344d7f","src/aggregate_device.rs":"7d2bd5f5fd7f3d008ebb69ad81f522ca0cb73db6d7b3e50ed1a63ea26ff721f4","src/audio_object.rs":"df10160d9fd83a2c23a49e69b78d39db3a9d6389607df6acfc05821293b6af5f","src/audio_unit.rs":"d783878930df4923b57ad230138c0f3fd6b0b9bb80a39725092ff4c6615162d8","src/cf_mutable_dict.rs":"fc42edd270c6dfb02f123214d2d8e487bbd62b5bd923b71eec13190fd0104d2a","src/dispatch.rs":"0f4b05076bf4ce8e9ce2a98c65149fcdd716b772a7ab111f37f9d12678552e1e","src/lib.rs":"bcc559d69ef6ed0cbea5b2a36fec89d8c011eb9da70e2f26c00f881ad97a2546","src/string.rs":"28f88b816c768bcfcc674a60d962b93f1c94e5e0f4cc8ed2a1301138b91039e7"},"package":null}
293 changes: 174 additions & 119 deletions third_party/rust/coreaudio-sys-utils/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,148 +4,203 @@ use std::ffi::CString;
use std::mem;
use std::os::raw::c_void;
use std::ptr;

pub const DISPATCH_QUEUE_SERIAL: dispatch_queue_attr_t = ptr::null_mut::<dispatch_queue_attr_s>();

pub fn create_dispatch_queue(
label: &'static str,
queue_attr: dispatch_queue_attr_t,
) -> dispatch_queue_t {
let label = CString::new(label).unwrap();
let c_string = label.as_ptr();
unsafe { dispatch_queue_create(c_string, queue_attr) }
}

pub fn release_dispatch_queue(queue: dispatch_queue_t) {
// TODO: This is incredibly unsafe. Find another way to release the queue.
unsafe {
dispatch_release(mem::transmute::<dispatch_queue_t, dispatch_object_t>(queue));
use std::sync::atomic::{AtomicBool, Ordering};

// Queue: A wrapper around `dispatch_queue_t`.
// ------------------------------------------------------------------------------------------------
#[derive(Debug)]
pub struct Queue(dispatch_queue_t);

impl Queue {
pub fn new(label: &str) -> Self {
const DISPATCH_QUEUE_SERIAL: dispatch_queue_attr_t =
ptr::null_mut::<dispatch_queue_attr_s>();
let label = CString::new(label).unwrap();
let c_string = label.as_ptr();
let queue = Self(unsafe { dispatch_queue_create(c_string, DISPATCH_QUEUE_SERIAL) });
queue.set_should_cancel(Box::new(AtomicBool::new(false)));
queue
}
}

pub fn async_dispatch<F>(queue: dispatch_queue_t, work: F)
where
F: Send + FnOnce(),
{
let (closure, executor) = create_closure_and_executor(work);
unsafe {
dispatch_async_f(queue, closure, executor);
}
}

pub fn sync_dispatch<F>(queue: dispatch_queue_t, work: F)
where
F: Send + FnOnce(),
{
let (closure, executor) = create_closure_and_executor(work);
unsafe {
dispatch_sync_f(queue, closure, executor);
}
}

// Return an raw pointer to a (unboxed) closure and an executor that
// will run the closure (after re-boxing the closure) when it's called.
fn create_closure_and_executor<F>(closure: F) -> (*mut c_void, dispatch_function_t)
where
F: FnOnce(),
{
extern "C" fn closure_executer<F>(unboxed_closure: *mut c_void)
pub fn run_async<F>(&self, work: F)
where
F: FnOnce(),
F: Send + FnOnce(),
{
// Retake the leaked closure.
let closure = unsafe { Box::from_raw(unboxed_closure as *mut F) };
// Execute the closure.
(*closure)();
// closure is released after finishing this function call.
let should_cancel = self.get_should_cancel();
let (closure, executor) = Self::create_closure_and_executor(|| {
if should_cancel.map_or(false, |v| v.load(Ordering::SeqCst)) {
return;
}
work();
});
unsafe {
dispatch_async_f(self.0, closure, executor);
}
}

let closure = Box::new(closure); // Allocate closure on heap.
let executor: dispatch_function_t = Some(closure_executer::<F>);

(
Box::into_raw(closure) as *mut c_void, // Leak the closure.
executor,
)
}

#[cfg(test)]
mod test {
use super::*;
use std::sync::{Arc, Mutex};
const COUNT: u32 = 10;

#[test]
fn test_async_dispatch() {
use std::sync::mpsc::channel;

get_queue_and_resource("Run with async dispatch api wrappers", |queue, resource| {
let (tx, rx) = channel();
for i in 0..COUNT {
let (res, tx) = (Arc::clone(&resource), tx.clone());
async_dispatch(queue, move || {
let mut res = res.lock().unwrap();
assert_eq!(res.last_touched, if i == 0 { None } else { Some(i - 1) });
assert_eq!(res.touched_count, i);
res.touch(i);
if i == COUNT - 1 {
tx.send(()).unwrap();
}
});
pub fn run_sync<F>(&self, work: F)
where
F: Send + FnOnce(),
{
let should_cancel = self.get_should_cancel();
let (closure, executor) = Self::create_closure_and_executor(|| {
if should_cancel.map_or(false, |v| v.load(Ordering::SeqCst)) {
return;
}
rx.recv().unwrap(); // Wait until it's touched COUNT times.
let resource = resource.lock().unwrap();
assert_eq!(resource.touched_count, COUNT);
assert_eq!(resource.last_touched.unwrap(), COUNT - 1);
work();
});
unsafe {
dispatch_sync_f(self.0, closure, executor);
}
}

#[test]
fn test_sync_dispatch() {
get_queue_and_resource("Run with sync dispatch api wrappers", |queue, resource| {
for i in 0..COUNT {
let res = Arc::clone(&resource);
sync_dispatch(queue, move || {
let mut res = res.lock().unwrap();
assert_eq!(res.last_touched, if i == 0 { None } else { Some(i - 1) });
assert_eq!(res.touched_count, i);
res.touch(i);
});
}
let resource = resource.lock().unwrap();
assert_eq!(resource.touched_count, COUNT);
assert_eq!(resource.last_touched.unwrap(), COUNT - 1);
pub fn run_final<F>(&self, work: F)
where
F: Send + FnOnce(),
{
let should_cancel = self.get_should_cancel();
let (closure, executor) = Self::create_closure_and_executor(|| {
work();
should_cancel
.expect("dispatch context should be allocated!")
.store(true, Ordering::SeqCst);
});
unsafe {
dispatch_sync_f(self.0, closure, executor);
}
}

struct Resource {
last_touched: Option<u32>,
touched_count: u32,
fn get_should_cancel(&self) -> Option<&mut AtomicBool> {
unsafe {
let context = dispatch_get_context(
mem::transmute::<dispatch_queue_t, dispatch_object_t>(self.0),
) as *mut AtomicBool;
context.as_mut()
}
}

impl Resource {
fn new() -> Self {
Resource {
last_touched: None,
touched_count: 0,
fn set_should_cancel(&self, context: Box<AtomicBool>) {
unsafe {
let queue = mem::transmute::<dispatch_queue_t, dispatch_object_t>(self.0);
// Leak the context from Box.
dispatch_set_context(queue, Box::into_raw(context) as *mut c_void);

extern "C" fn finalizer(context: *mut c_void) {
// Retake the leaked context into box and then drop it.
let _ = unsafe { Box::from_raw(context as *mut AtomicBool) };
}

// The `finalizer` is only run if the `context` in `queue` is set by `dispatch_set_context`.
dispatch_set_finalizer_f(queue, Some(finalizer));
}
fn touch(&mut self, who: u32) {
self.last_touched = Some(who);
self.touched_count += 1;
}

fn release(&self) {
unsafe {
// This will release the inner `dispatch_queue_t` asynchronously.
// TODO: It's incredibly unsafe to call `transmute` directly.
// Find another way to release the queue.
dispatch_release(mem::transmute::<dispatch_queue_t, dispatch_object_t>(
self.0,
));
}
}

fn get_queue_and_resource<F>(label: &'static str, callback: F)
fn create_closure_and_executor<F>(closure: F) -> (*mut c_void, dispatch_function_t)
where
F: FnOnce(dispatch_queue_t, Arc<Mutex<Resource>>),
F: FnOnce(),
{
let queue = create_dispatch_queue(label, DISPATCH_QUEUE_SERIAL);
let resource = Arc::new(Mutex::new(Resource::new()));
extern "C" fn closure_executer<F>(unboxed_closure: *mut c_void)
where
F: FnOnce(),
{
// Retake the leaked closure.
let closure = unsafe { Box::from_raw(unboxed_closure as *mut F) };
// Execute the closure.
(*closure)();
// closure is released after finishing this function call.
}

let closure = Box::new(closure); // Allocate closure on heap.
let executor: dispatch_function_t = Some(closure_executer::<F>);

(
Box::into_raw(closure) as *mut c_void, // Leak the closure.
executor,
)
}
}

impl Drop for Queue {
fn drop(&mut self) {
self.release();
}
}

callback(queue, resource);
impl Clone for Queue {
fn clone(&self) -> Self {
// TODO: It's incredibly unsafe to call `transmute` directly.
// Find another way to release the queue.
unsafe {
dispatch_retain(mem::transmute::<dispatch_queue_t, dispatch_object_t>(
self.0,
));
}
Self(self.0)
}
}

#[test]
fn run_tasks_in_order() {
let mut visited = Vec::<u32>::new();

// Rust compilter doesn't allow a pointer to be passed across threads.
// A hacky way to do that is to cast the pointer into a value, then
// the value, which is actually an address, can be copied into threads.
let ptr = &mut visited as *mut Vec<u32> as usize;

fn visit(v: u32, visited_ptr: usize) {
let visited = unsafe { &mut *(visited_ptr as *mut Vec<u32>) };
visited.push(v);
};

let queue = Queue::new("Run tasks in order");

// Release the queue.
release_dispatch_queue(queue);
queue.run_sync(move || visit(1, ptr));
queue.run_sync(move || visit(2, ptr));
queue.run_async(move || visit(3, ptr));
queue.run_async(move || visit(4, ptr));
// Call sync here to block the current thread and make sure all the tasks are done.
queue.run_sync(move || visit(5, ptr));

assert_eq!(visited, vec![1, 2, 3, 4, 5]);
}

#[test]
fn run_final_task() {
let mut visited = Vec::<u32>::new();

{
// Rust compilter doesn't allow a pointer to be passed across threads.
// A hacky way to do that is to cast the pointer into a value, then
// the value, which is actually an address, can be copied into threads.
let ptr = &mut visited as *mut Vec<u32> as usize;

fn visit(v: u32, visited_ptr: usize) {
let visited = unsafe { &mut *(visited_ptr as *mut Vec<u32>) };
visited.push(v);
};

let queue = Queue::new("Task after run_final will be cancelled");

queue.run_sync(move || visit(1, ptr));
queue.run_async(move || visit(2, ptr));
queue.run_final(move || visit(3, ptr));
queue.run_async(move || visit(4, ptr));
queue.run_sync(move || visit(5, ptr));
}
// `queue` will be dropped asynchronously and then the `finalizer` of the `queue`
// should be fired to clean up the `context` set in the `queue`.

assert_eq!(visited, vec![1, 2, 3]);
}
2 changes: 1 addition & 1 deletion third_party/rust/cubeb-coreaudio/.cargo-checksum.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"files":{".editorconfig":"4e53b182bcc78b83d7e1b5c03efa14d22d4955c4ed2514d1ba4e99c1eb1a50ba",".travis.yml":"878d9519da0844cd3de2999f81fce966452f0fb65150605c25a1abf3523360ab","Cargo.toml":"bcb3ec3785c3cbe799bb41c07176afdd0028328be79932a4e44bc97a364e8c69","LICENSE":"6e6f56aff5bbf3cbc60747e152fb1a719bd0716aaf6d711c554f57d92e96297c","README.md":"fa323b7386a8a0c75478b77a30a3c5d33f1df23d9775b97570fa0501ef784e95","install_rustfmt_clippy.sh":"4ae90d8dcb9757cb3ae4ae142ef80e5377c0dde61c63f4a3c32418646e80ca7b","run_sanitizers.sh":"913efec447b4c37751af93eef4a974f293426c20c720d37c5bbe69e955d105eb","run_tests.sh":"fcdae57b81426c5dbfc8873a9ff1ac07924ed0717f822b50a67934cac2e9d4f1","src/backend/aggregate_device.rs":"ae21129aa6b3c7bd3376751b6a94d1ebe6c9f7afcd1db3107fb4d703d04da6b3","src/backend/auto_array.rs":"5f35545baba2b005e13a2225bd1cbdd94ffc2097554d61479929bfc5442a6dd6","src/backend/auto_release.rs":"050fdcee74cf46b9a8a85a877e166d72a853d33220f59cf734cbb6ea09daa441","src/backend/device_property.rs":"324a7c9672daa49ee2221a56d140e1c8298719dab66f204b19d10f3632f96602","src/backend/mixer.rs":"0c237bd5ca63b028c4b22ddc5bc026d7e21c0fa9b4e337f00b6131ed0a0806a5","src/backend/mod.rs":"250cf24aabe1f1e9e8eafd375a538554523accaa6b514588b157d0c413d39b86","src/backend/resampler.rs":"fd1281d28a4db1659d2f75e43b8457651745e1b6eb5a53a77f04d752135f6dc7","src/backend/tests/aggregate_device.rs":"107f5c637844cd5ae43d2b42cec4ef3369bb702751586078c0a9d50f039161cd","src/backend/tests/api.rs":"d81128f0982ecc816666e91f57e722cc21ba9cc09a2a36103acc7c981f57b36d","src/backend/tests/backlog.rs":"3b189a7e036543c467cc242af0ed3332721179ee2b1c8847a6db563546f1ac52","src/backend/tests/device_change.rs":"2138e7ed4721872ce3a41f3652bfa4e7eca97fd136473af6472313c61ff24ed3","src/backend/tests/device_property.rs":"b1a9ae79aa5b9a3f180040d0ef0954b134680d586882d2062c5e017b555bff57","src/backend/tests/interfaces.rs":"14943e84a79976a7ef52882edeb9330350705d5190db6647f98b4ffa851a8396","src/backend/tests/manual.rs":"dc707836dab31f83d4b325afbc4dc4c8104ac8036e87f59ade3309ee83fe2d3f","src/backend/tests/mod.rs":"8dba770023d7f9c4228f0e11915347f0e07da5fd818e3ee4478c4b197af9aa2a","src/backend/tests/parallel.rs":"f9e1883660d6146b6e5075806561f5f689810e25c5e7764dfd28c9b939821a49","src/backend/tests/tone.rs":"16150438317ce501986734167b5fb97bfec567228acbcd8f3b4c4484c22f29e0","src/backend/tests/utils.rs":"1bb99ef71d3c18545bca49767e7b6bfffbe11896246994f41be7ed372772fd48","src/backend/utils.rs":"5ce1b753af0ffb654b6b2038d649aea88eebd27390a607a6d5988a9e40a4a717","src/capi.rs":"21b66b70545bf04ec719928004d1d9adb45b24ced51288f5b2993d79aaf78f5f","src/lib.rs":"5e586d45cd6b3722f0a6736d9252593299269817a153eef1930a5fb9bfbb56f5","todo.md":"29545b4d9c516396f82bd392797e2713d4602036eaba0f151b384af764f8515f"},"package":null}
{"files":{".editorconfig":"4e53b182bcc78b83d7e1b5c03efa14d22d4955c4ed2514d1ba4e99c1eb1a50ba",".travis.yml":"878d9519da0844cd3de2999f81fce966452f0fb65150605c25a1abf3523360ab","Cargo.toml":"bcb3ec3785c3cbe799bb41c07176afdd0028328be79932a4e44bc97a364e8c69","LICENSE":"6e6f56aff5bbf3cbc60747e152fb1a719bd0716aaf6d711c554f57d92e96297c","README.md":"0d5a4c39e737aeeccfd5a54914e8927631ad86e3da8c24f62594f2f89a3302bf","install_rustfmt_clippy.sh":"4ae90d8dcb9757cb3ae4ae142ef80e5377c0dde61c63f4a3c32418646e80ca7b","run_device_tests.sh":"e66d32da5439818eee30fc5e7ed9ab9990723ffafe684870efe82dac11be07ef","run_sanitizers.sh":"54970203e86eed00245de3d9b53584d5d8868be983f90b27c603b310ba5554e5","run_tests.sh":"257ac9b30e0ff3775d713668a9704f15ad4ed2334aca7d8c544d079cde197e8a","src/backend/aggregate_device.rs":"ae21129aa6b3c7bd3376751b6a94d1ebe6c9f7afcd1db3107fb4d703d04da6b3","src/backend/auto_array.rs":"5f35545baba2b005e13a2225bd1cbdd94ffc2097554d61479929bfc5442a6dd6","src/backend/auto_release.rs":"050fdcee74cf46b9a8a85a877e166d72a853d33220f59cf734cbb6ea09daa441","src/backend/device_property.rs":"324a7c9672daa49ee2221a56d140e1c8298719dab66f204b19d10f3632f96602","src/backend/mixer.rs":"0c237bd5ca63b028c4b22ddc5bc026d7e21c0fa9b4e337f00b6131ed0a0806a5","src/backend/mod.rs":"e6ed4937f9ca52626a0ab6728631a88c9ae1369fd1ff61a377dfa64c3d406fb3","src/backend/resampler.rs":"fd1281d28a4db1659d2f75e43b8457651745e1b6eb5a53a77f04d752135f6dc7","src/backend/tests/aggregate_device.rs":"107f5c637844cd5ae43d2b42cec4ef3369bb702751586078c0a9d50f039161cd","src/backend/tests/api.rs":"58cbf67e3f4588f5e644ebcdc24942fcd6299c26f42a7e27fb4b6b9d8e5245f8","src/backend/tests/backlog.rs":"3b189a7e036543c467cc242af0ed3332721179ee2b1c8847a6db563546f1ac52","src/backend/tests/device_change.rs":"8261f561f69dabd374ac47d69aa484812b65070a9e9581dfb2605e8404eaad6d","src/backend/tests/device_property.rs":"b1a9ae79aa5b9a3f180040d0ef0954b134680d586882d2062c5e017b555bff57","src/backend/tests/interfaces.rs":"14943e84a79976a7ef52882edeb9330350705d5190db6647f98b4ffa851a8396","src/backend/tests/manual.rs":"dc707836dab31f83d4b325afbc4dc4c8104ac8036e87f59ade3309ee83fe2d3f","src/backend/tests/mod.rs":"8dba770023d7f9c4228f0e11915347f0e07da5fd818e3ee4478c4b197af9aa2a","src/backend/tests/parallel.rs":"f9e1883660d6146b6e5075806561f5f689810e25c5e7764dfd28c9b939821a49","src/backend/tests/tone.rs":"16150438317ce501986734167b5fb97bfec567228acbcd8f3b4c4484c22f29e0","src/backend/tests/utils.rs":"1bb99ef71d3c18545bca49767e7b6bfffbe11896246994f41be7ed372772fd48","src/backend/utils.rs":"5ce1b753af0ffb654b6b2038d649aea88eebd27390a607a6d5988a9e40a4a717","src/capi.rs":"21b66b70545bf04ec719928004d1d9adb45b24ced51288f5b2993d79aaf78f5f","src/lib.rs":"5e586d45cd6b3722f0a6736d9252593299269817a153eef1930a5fb9bfbb56f5","todo.md":"29545b4d9c516396f82bd392797e2713d4602036eaba0f151b384af764f8515f"},"package":null}
Loading

0 comments on commit 4c786ba

Please sign in to comment.