diff --git a/.cargo/config.in b/.cargo/config.in index 8698338d0a99..3b529188959b 100644 --- a/.cargo/config.in +++ b/.cargo/config.in @@ -45,7 +45,7 @@ rev = "ad56ea14ac915f1e7ecbcf6ac38182443b0dd29e" [source."https://github.com/mozilla/audioipc-2"] git = "https://github.com/mozilla/audioipc-2" replace-with = "vendored-sources" -rev = "ca5abc4bb056e18e4b8e5b2bb5ed9a8a21443ee7" +rev = "8fb5ff19fba7b09e8e66598122421e68a5c573ac" [source."https://github.com/mozilla/application-services"] git = "https://github.com/mozilla/application-services" diff --git a/Cargo.lock b/Cargo.lock index cd5c704dc743..ddeeddf525d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -139,7 +139,7 @@ dependencies = [ [[package]] name = "audioipc" version = "0.2.5" -source = "git+https://github.com/mozilla/audioipc-2?rev=ca5abc4bb056e18e4b8e5b2bb5ed9a8a21443ee7#ca5abc4bb056e18e4b8e5b2bb5ed9a8a21443ee7" +source = "git+https://github.com/mozilla/audioipc-2?rev=8fb5ff19fba7b09e8e66598122421e68a5c573ac#8fb5ff19fba7b09e8e66598122421e68a5c573ac" dependencies = [ "audio_thread_priority", "bincode", @@ -167,7 +167,7 @@ dependencies = [ [[package]] name = "audioipc-client" version = "0.4.0" -source = "git+https://github.com/mozilla/audioipc-2?rev=ca5abc4bb056e18e4b8e5b2bb5ed9a8a21443ee7#ca5abc4bb056e18e4b8e5b2bb5ed9a8a21443ee7" +source = "git+https://github.com/mozilla/audioipc-2?rev=8fb5ff19fba7b09e8e66598122421e68a5c573ac#8fb5ff19fba7b09e8e66598122421e68a5c573ac" dependencies = [ "audio_thread_priority", "audioipc", @@ -181,7 +181,7 @@ dependencies = [ [[package]] name = "audioipc-server" version = "0.2.3" -source = "git+https://github.com/mozilla/audioipc-2?rev=ca5abc4bb056e18e4b8e5b2bb5ed9a8a21443ee7#ca5abc4bb056e18e4b8e5b2bb5ed9a8a21443ee7" +source = "git+https://github.com/mozilla/audioipc-2?rev=8fb5ff19fba7b09e8e66598122421e68a5c573ac#8fb5ff19fba7b09e8e66598122421e68a5c573ac" dependencies = [ "audio_thread_priority", "audioipc", diff --git a/third_party/rust/audioipc-client/.cargo-checksum.json b/third_party/rust/audioipc-client/.cargo-checksum.json index d19119e04f8c..4afdfd333f29 100644 --- a/third_party/rust/audioipc-client/.cargo-checksum.json +++ b/third_party/rust/audioipc-client/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"4020e8c4119327dac49b47391c902eb69bb927c9e7d05f5882ad9e84cff4ec5e","cbindgen.toml":"bd89c5a9f52395b1c703ff04d1c0019dc3c92b691d571ae503c4b85753a44a39","src/context.rs":"53fa8876e0288ad606fa4e9593a4508ee1382bc8f1b9dde53c2fe9e820d3f9e4","src/lib.rs":"ad38115edb187bcc7e5c57a355913f4f588ffebabc6f77b314b0b1f503886590","src/send_recv.rs":"450bdb1d8a346634c0237f2081b424d11e2c19ad81670009303f8a03b3bfb196","src/stream.rs":"87d78d291f63302240571687f1303b166f9be2a93a49ccea9b546c2d0eff62df"},"package":null} \ No newline at end of file +{"files":{"Cargo.toml":"4020e8c4119327dac49b47391c902eb69bb927c9e7d05f5882ad9e84cff4ec5e","cbindgen.toml":"bd89c5a9f52395b1c703ff04d1c0019dc3c92b691d571ae503c4b85753a44a39","src/context.rs":"a9bbd35faf15a579e92e56bc78881b4a064edb579f875b8804db1737fd2fdfeb","src/lib.rs":"ad38115edb187bcc7e5c57a355913f4f588ffebabc6f77b314b0b1f503886590","src/send_recv.rs":"450bdb1d8a346634c0237f2081b424d11e2c19ad81670009303f8a03b3bfb196","src/stream.rs":"9ef8ea1ccf33429f5ab63e0029e8818382f3f4d01617da3844fcd79d9ea1704e"},"package":null} \ No newline at end of file diff --git a/third_party/rust/audioipc-client/src/context.rs b/third_party/rust/audioipc-client/src/context.rs index 7441d0c0e081..b04d2cc263e7 100644 --- a/third_party/rust/audioipc-client/src/context.rs +++ b/third_party/rust/audioipc-client/src/context.rs @@ -383,16 +383,12 @@ impl ContextOps for ClientContext { ContextSetupDeviceCollectionCallback => ContextSetupDeviceCollectionCallback())?; + // TODO: The lowest comms layer expects exactly 3 PlatformHandles, but we only + // need one here. The server sent two dummy valid handles, ignore those (closed on drop) + // and use the one we need. let stream = unsafe { audioipc::MessageStream::from_raw_fd(fds.platform_handles[0].into_raw()) }; - // TODO: The lowest comms layer expects exactly 3 PlatformHandles, but we only - // need one here. Drop the dummy handles the other side sent us to discard. - unsafe { - fds.platform_handles[1].into_file(); - fds.platform_handles[2].into_file(); - } - let server = DeviceCollectionServer { input_device_callback: self.input_device_callback.clone(), output_device_callback: self.output_device_callback.clone(), diff --git a/third_party/rust/audioipc-client/src/stream.rs b/third_party/rust/audioipc-client/src/stream.rs index d89900dfecee..da5756fe8443 100644 --- a/third_party/rust/audioipc-client/src/stream.rs +++ b/third_party/rust/audioipc-client/src/stream.rs @@ -8,7 +8,7 @@ use crate::{assert_not_in_callback, run_in_callback}; use audioipc::frame::{framed, Framed}; use audioipc::messages::{self, CallbackReq, CallbackResp, ClientMessage, ServerMessage}; use audioipc::rpc; -use audioipc::shm::{SharedMemMutSlice, SharedMemSlice}; +use audioipc::shm::SharedMem; use audioipc::{codec::LengthDelimitedCodec, messages::StreamCreateParams}; use cubeb_backend::{ffi, DeviceRef, Error, Result, Stream, StreamOps}; use futures::Future; @@ -51,8 +51,8 @@ pub struct ClientStream<'ctx> { } struct CallbackServer { - input_shm: Option, - output_shm: Option, + input_shm: Option, + output_shm: Option, data_cb: ffi::cubeb_data_callback, state_cb: ffi::cubeb_state_callback, user_ptr: usize, @@ -85,31 +85,32 @@ impl rpc::Server for CallbackServer { // Clone values that need to be moved into the cpu pool thread. let input_shm = match self.input_shm { - Some(ref shm) => unsafe { Some(shm.unsafe_clone()) }, + Some(ref shm) => unsafe { Some(shm.unsafe_view()) }, None => None, }; - let mut output_shm = match self.output_shm { - Some(ref shm) => unsafe { Some(shm.unsafe_clone()) }, + let output_shm = match self.output_shm { + Some(ref shm) => unsafe { Some(shm.unsafe_view()) }, None => None, }; let user_ptr = self.user_ptr; let cb = self.data_cb.unwrap(); self.cpu_pool.spawn_fn(move || { - // TODO: This is proof-of-concept. Make it better. - let input_ptr: *const u8 = match input_shm { - Some(shm) => shm - .get_slice(nframes as usize * input_frame_size) - .unwrap() - .as_ptr(), + let input_ptr = match input_shm { + Some(shm) => unsafe { + shm.get_slice(nframes as usize * input_frame_size) + .unwrap() + .as_ptr() + }, None => ptr::null(), }; - let output_ptr: *mut u8 = match output_shm { - Some(ref mut shm) => shm - .get_mut_slice(nframes as usize * output_frame_size) - .unwrap() - .as_mut_ptr(), - None => ptr::null_mut(), + let output_ptr = match output_shm { + Some(mut shm) => unsafe { + shm.get_mut_slice(nframes as usize * output_frame_size) + .unwrap() + .as_mut_ptr() + }, + None => ptr::null(), }; run_in_callback(|| { @@ -189,9 +190,8 @@ impl<'ctx> ClientStream<'ctx> { let stream = unsafe { audioipc::MessageStream::from_raw_fd(data.platform_handles[0].into_raw()) }; - let input_file = unsafe { data.platform_handles[1].into_file() }; let input_shm = if has_input { - match SharedMemSlice::from(&input_file, audioipc::SHM_AREA_SIZE) { + match unsafe { SharedMem::from(&data.platform_handles[1], audioipc::SHM_AREA_SIZE) } { Ok(shm) => Some(shm), Err(e) => { debug!("Client failed to set up input shmem: {}", e); @@ -202,9 +202,8 @@ impl<'ctx> ClientStream<'ctx> { None }; - let output_file = unsafe { data.platform_handles[2].into_file() }; let output_shm = if has_output { - match SharedMemMutSlice::from(&output_file, audioipc::SHM_AREA_SIZE) { + match unsafe { SharedMem::from(&data.platform_handles[2], audioipc::SHM_AREA_SIZE) } { Ok(shm) => Some(shm), Err(e) => { debug!("Client failed to set up output shmem: {}", e); diff --git a/third_party/rust/audioipc-server/.cargo-checksum.json b/third_party/rust/audioipc-server/.cargo-checksum.json index 326a2e7ab5a7..b5e6e9adc5ed 100644 --- a/third_party/rust/audioipc-server/.cargo-checksum.json +++ b/third_party/rust/audioipc-server/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"6329179497fb654bec0dea9f3642056309de3fa37d4042a48d18224e7b4742d3","cbindgen.toml":"bd89c5a9f52395b1c703ff04d1c0019dc3c92b691d571ae503c4b85753a44a39","src/lib.rs":"c1756c15724af6639cad7c3e3e730803323ac0357870262c835ccc9430ec457e","src/server.rs":"342928ba26c617155118004fbd30576e340082de90405fe25505d51555c8cfa5"},"package":null} \ No newline at end of file +{"files":{"Cargo.toml":"6329179497fb654bec0dea9f3642056309de3fa37d4042a48d18224e7b4742d3","cbindgen.toml":"bd89c5a9f52395b1c703ff04d1c0019dc3c92b691d571ae503c4b85753a44a39","src/lib.rs":"c1756c15724af6639cad7c3e3e730803323ac0357870262c835ccc9430ec457e","src/server.rs":"bbb7feac3daad9306aca572c4fb8471f99cef9e80914f6f860dbd3403151efcc"},"package":null} \ No newline at end of file diff --git a/third_party/rust/audioipc-server/src/server.rs b/third_party/rust/audioipc-server/src/server.rs index 54404343d6a0..b7f3c7ae233b 100644 --- a/third_party/rust/audioipc-server/src/server.rs +++ b/third_party/rust/audioipc-server/src/server.rs @@ -14,7 +14,7 @@ use audioipc::messages::{ }; use audioipc::platformhandle_passing::FramedWithPlatformHandles; use audioipc::rpc; -use audioipc::shm::{SharedMemReader, SharedMemWriter}; +use audioipc::shm::SharedMem; use audioipc::{MessageStream, PlatformHandle}; use cubeb_core as cubeb; use cubeb_core::ffi; @@ -231,9 +231,9 @@ struct ServerStreamCallbacks { /// Size of output frame in bytes output_frame_size: u16, /// Shared memory buffer for sending input data to client - input_shm: SharedMemWriter, + input_shm: Option, /// Shared memory buffer for receiving output data from client - output_shm: SharedMemReader, + output_shm: Option, /// RPC interface to callback server running in client rpc: rpc::ClientProxy, } @@ -247,7 +247,13 @@ impl ServerStreamCallbacks { output.len() ); - self.input_shm.write(input).unwrap(); + unsafe { + if let Some(shm) = &mut self.input_shm { + shm.get_mut_slice(input.len()) + .unwrap() + .copy_from_slice(input); + } + } let r = self .rpc @@ -263,7 +269,11 @@ impl ServerStreamCallbacks { if frames >= 0 { let nbytes = frames as usize * self.output_frame_size as usize; trace!("Reslice output to {}", nbytes); - self.output_shm.read(&mut output[..nbytes]).unwrap(); + unsafe { + if let Some(shm) = &self.output_shm { + &mut output[..nbytes].copy_from_slice(shm.get_slice(nbytes).unwrap()); + } + } } frames } @@ -677,9 +687,9 @@ impl CubebServer { debug!("Created callback pair: {:?}-{:?}", ipc_server, ipc_client); let shm_id = get_shm_id(); let (input_shm, input_file) = - SharedMemWriter::new(&format!("{}-input", shm_id), audioipc::SHM_AREA_SIZE)?; + SharedMem::new(&format!("{}-input", shm_id), audioipc::SHM_AREA_SIZE)?; let (output_shm, output_file) = - SharedMemReader::new(&format!("{}-output", shm_id), audioipc::SHM_AREA_SIZE)?; + SharedMem::new(&format!("{}-output", shm_id), audioipc::SHM_AREA_SIZE)?; // This code is currently running on the Client/Server RPC // handling thread. We need to move the registration of the @@ -702,6 +712,11 @@ impl CubebServer { Err(_) => bail!("Failed to create callback rpc."), }; + // TODO: The lowest comms layer expects exactly 3 PlatformHandles, so we always configure both sides of the shm. + // ServerStreamCallbacks only needs the active shm, so drop any unused shm now. + let input_shm = params.input_stream_params.and(Some(input_shm)); + let output_shm = params.output_stream_params.and(Some(output_shm)); + let cbs = Box::new(ServerStreamCallbacks { input_frame_size, output_frame_size, @@ -718,11 +733,7 @@ impl CubebServer { Ok(ClientMessage::StreamCreated(StreamCreate { token: key, - platform_handles: [ - PlatformHandle::from(ipc_client), - PlatformHandle::from(input_file), - PlatformHandle::from(output_file), - ], + platform_handles: [PlatformHandle::from(ipc_client), input_file, output_file], target_pid: self.remote_pid.unwrap(), })) } @@ -772,7 +783,11 @@ impl CubebServer { ); match stream { Ok(stream) => stream, - Err(e) => return Err(e.into()), // XXX full teardown of ServerStream? + Err(e) => { + debug!("Unregistering stream {:?} (stream error {:?})", stm_tok, e); + self.streams.remove(stm_tok); + return Err(e.into()); + } } }; diff --git a/third_party/rust/audioipc/.cargo-checksum.json b/third_party/rust/audioipc/.cargo-checksum.json index bfd12daf422e..b0060527ebfc 100644 --- a/third_party/rust/audioipc/.cargo-checksum.json +++ b/third_party/rust/audioipc/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"14e6f5f7e40b7838642e61d45ab49914982736e673711f8a90540b79a82f8c91","build.rs":"112a5167341ca2148f0a64581132808b8a0bab34ceafad5236ad90c43eceea92","src/async_msg.rs":"37ba3ae4b41c14592dbb6333c8ad7faf4c84e05ef3b7d91a53249a7efb2e0f6a","src/cmsg.rs":"929977c03274c798c4295bc7887f05efdfd130e6b7665200a44276b4989ce156","src/cmsghdr.c":"2e81c826aa92931fd1410b5beca9ee6d2afdc91e3eb29991d3a935766f2cb357","src/codec.rs":"3951afd310af5d822488329f816482443959f2b744e231d82ad122884de15dbd","src/core.rs":"721de353d3b0b5126bf5b25cfb1f99244702309ce9f9f24cc2ce3c5858228794","src/errors.rs":"67a4a994d0724397657581cde153bdfc05ce86e7efc467f23fafc8f64df80fa4","src/fd_passing.rs":"47f3471f3e1bfe7c1c505d96051dc9c02889b14ba79ce589854062f789e7fc16","src/frame.rs":"7f9547a42346c53e710d8d9e41ae824e6d3e105a2fcbec0afd372e27e321ad0e","src/handle_passing.rs":"f381817bdfd8805d1dd966da0929d7cec7899560d2d79e1c8495ccbc6e067c77","src/lib.rs":"c48ec2a49c7132ad2af44ae0a953d74d3d71e2909661ab54b328e92d2b8d9861","src/messages.rs":"6b642cab6a7133b6bead891b21678e4d029b8bea921000a59478ee2fabbbe808","src/messagestream_unix.rs":"ab18b902a6bde57a028b2cc91a85d68b100cc443f87f23898bc6df01bb965332","src/messagestream_win.rs":"cf614258218d0e73188d18a865e33dc69159b76d72b95117e20b1b84881ee07c","src/msg.rs":"f5353e942f7818742190541e568685d6b4d6200b55bfc60e46ee3db05f802436","src/rpc/client/mod.rs":"04e80b689548e7888b34441a7224dfa8cf557b8b4164754daee95a95b76f9aee","src/rpc/client/proxy.rs":"8d9c9b38ecec4ab5ee3b6e4c2d7aea9dbb4f7cf5c25d39a5db0c76aa41008497","src/rpc/driver.rs":"d6de7f6fcde141c1378db73bc8ec4da187cc414f84505ec1b0233416b734917a","src/rpc/mod.rs":"3b14af0be2b4c7b30a0dab9cca353e092652a16e29002f5aeba24dca45e33d1e","src/rpc/server.rs":"7caf0b2d659783b4c5c9dd9efe4cb9a2e7d5955c0dfda3d2e79581116bb9334b","src/shm.rs":"ef51fb868f3c8686a6bd29b545b1410d53c2c413efca28d2e492fdb224c58d76","src/tokio_named_pipes.rs":"4023bfc79bf8ade5c3c1e2551638aae78b6b1cc1d4ef8dad352a021a0e4459aa","src/tokio_uds_stream.rs":"3251b91e4129f174a588648ec43575b35e139b67d4b8833fe324e82e67d5c3da"},"package":null} \ No newline at end of file +{"files":{"Cargo.toml":"c4ca8cb8a0b9b354c8d6c5949f63011666b968555539695a57d077d0b1f6a813","build.rs":"112a5167341ca2148f0a64581132808b8a0bab34ceafad5236ad90c43eceea92","src/async_msg.rs":"37ba3ae4b41c14592dbb6333c8ad7faf4c84e05ef3b7d91a53249a7efb2e0f6a","src/cmsg.rs":"929977c03274c798c4295bc7887f05efdfd130e6b7665200a44276b4989ce156","src/cmsghdr.c":"2e81c826aa92931fd1410b5beca9ee6d2afdc91e3eb29991d3a935766f2cb357","src/codec.rs":"3951afd310af5d822488329f816482443959f2b744e231d82ad122884de15dbd","src/core.rs":"721de353d3b0b5126bf5b25cfb1f99244702309ce9f9f24cc2ce3c5858228794","src/errors.rs":"67a4a994d0724397657581cde153bdfc05ce86e7efc467f23fafc8f64df80fa4","src/fd_passing.rs":"46ea978560317415929d400256bdff8cf7eb2a9428e44849c855354b580efc1a","src/frame.rs":"7f9547a42346c53e710d8d9e41ae824e6d3e105a2fcbec0afd372e27e321ad0e","src/handle_passing.rs":"0d9f1fb234f8dac8d8249aef03c6b7ef073cde12804a3fa7c393b6d6104254ab","src/lib.rs":"4d51b8209d4cc0fdafd228f4b62e7afd4ab128ff72d8fb02a4aeb87a304ce74f","src/messages.rs":"6b642cab6a7133b6bead891b21678e4d029b8bea921000a59478ee2fabbbe808","src/messagestream_unix.rs":"ab18b902a6bde57a028b2cc91a85d68b100cc443f87f23898bc6df01bb965332","src/messagestream_win.rs":"cf614258218d0e73188d18a865e33dc69159b76d72b95117e20b1b84881ee07c","src/msg.rs":"f5353e942f7818742190541e568685d6b4d6200b55bfc60e46ee3db05f802436","src/rpc/client/mod.rs":"04e80b689548e7888b34441a7224dfa8cf557b8b4164754daee95a95b76f9aee","src/rpc/client/proxy.rs":"8d9c9b38ecec4ab5ee3b6e4c2d7aea9dbb4f7cf5c25d39a5db0c76aa41008497","src/rpc/driver.rs":"d6de7f6fcde141c1378db73bc8ec4da187cc414f84505ec1b0233416b734917a","src/rpc/mod.rs":"3b14af0be2b4c7b30a0dab9cca353e092652a16e29002f5aeba24dca45e33d1e","src/rpc/server.rs":"7caf0b2d659783b4c5c9dd9efe4cb9a2e7d5955c0dfda3d2e79581116bb9334b","src/shm.rs":"e5d53d0c03ef25d669addb91870aa5a50dccec279f9dc50312ca6189d6f5d558","src/tokio_named_pipes.rs":"4023bfc79bf8ade5c3c1e2551638aae78b6b1cc1d4ef8dad352a021a0e4459aa","src/tokio_uds_stream.rs":"3251b91e4129f174a588648ec43575b35e139b67d4b8833fe324e82e67d5c3da"},"package":null} \ No newline at end of file diff --git a/third_party/rust/audioipc/Cargo.toml b/third_party/rust/audioipc/Cargo.toml index 38ef4ae4a97a..9606990ac5c3 100644 --- a/third_party/rust/audioipc/Cargo.toml +++ b/third_party/rust/audioipc/Cargo.toml @@ -15,7 +15,6 @@ bytes = "0.4" cubeb = "0.9" futures = "0.1.29" log = "0.4" -memmap = "0.7" serde = "1" serde_derive = "1" tokio = "0.1" @@ -28,6 +27,7 @@ libc = "0.2" mio = "0.6.19" mio-uds = "0.6.7" tokio-reactor = "0.1" +memmap = "0.7" [target.'cfg(windows)'.dependencies] mio = "0.6.19" diff --git a/third_party/rust/audioipc/src/fd_passing.rs b/third_party/rust/audioipc/src/fd_passing.rs index faf29e2dd8cb..40b4bd87c469 100644 --- a/third_party/rust/audioipc/src/fd_passing.rs +++ b/third_party/rust/audioipc/src/fd_passing.rs @@ -236,8 +236,11 @@ where } } + // Need to take fd ownership here for `set_frame` to keep fds alive until `do_write`, + // otherwise fds are closed too early (when `item` is dropped). let fds = item.platform_handles(); self.codec.encode(item, &mut self.write_buf)?; + let fds = fds.and_then(|fds| { cmsg::builder(&mut self.outgoing_fds) .rights(&fds.0[..]) diff --git a/third_party/rust/audioipc/src/handle_passing.rs b/third_party/rust/audioipc/src/handle_passing.rs index 347613e5f554..ad5e14cd0290 100644 --- a/third_party/rust/audioipc/src/handle_passing.rs +++ b/third_party/rust/audioipc/src/handle_passing.rs @@ -175,16 +175,17 @@ where } } - let mut got_handles = false; - if let Some((handles, target_pid)) = item.platform_handles() { - got_handles = true; + // Take handle ownership here. + let handles = item.platform_handles(); + if let Some((handles, target_pid)) = handles { + // TODO: This could leak target handles if a duplicate fails - make this more robust. let remote_handles = unsafe { // Attempt to duplicate all 3 handles before checking // result, since we rely on duplicate_platformhandle closing // our source handles. - let r1 = duplicate_platformhandle(handles[0], target_pid); - let r2 = duplicate_platformhandle(handles[1], target_pid); - let r3 = duplicate_platformhandle(handles[2], target_pid); + let r1 = duplicate_platformhandle(handles[0], Some(target_pid), true); + let r2 = duplicate_platformhandle(handles[1], Some(target_pid), true); + let r3 = duplicate_platformhandle(handles[2], Some(target_pid), true); [r1?, r2?, r3?] }; trace!( @@ -197,7 +198,7 @@ where self.codec.encode(item, &mut self.write_buf)?; - if got_handles { + if handles.is_some() { // Enforce splitting sends on messages that contain file // descriptors. self.set_frame(); @@ -241,23 +242,30 @@ use super::PlatformHandleType; use winapi::shared::minwindef::{DWORD, FALSE}; use winapi::um::{handleapi, processthreadsapi, winnt}; -// source_handle is effectively taken ownership of (consumed) and -// closed when duplicate_platformhandle is called. -// TODO: Make this transfer more explicit via the type system. -unsafe fn duplicate_platformhandle( +pub(crate) unsafe fn duplicate_platformhandle( source_handle: PlatformHandleType, - target_pid: DWORD, + target_pid: Option, + close_source: bool, ) -> Result { let source = processthreadsapi::GetCurrentProcess(); - let target = processthreadsapi::OpenProcess(winnt::PROCESS_DUP_HANDLE, FALSE, target_pid); - if !super::valid_handle(target) { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - "invalid target process", - )); - } + let target = if let Some(pid) = target_pid { + let target = processthreadsapi::OpenProcess(winnt::PROCESS_DUP_HANDLE, FALSE, pid); + if !super::valid_handle(target) { + return Err(std::io::Error::new( + std::io::ErrorKind::Other, + "invalid target process", + )); + } + target + } else { + source + }; let mut target_handle = std::ptr::null_mut(); + let mut options = winnt::DUPLICATE_SAME_ACCESS; + if close_source { + options |= winnt::DUPLICATE_CLOSE_SOURCE; + } let ok = handleapi::DuplicateHandle( source, source_handle, @@ -265,7 +273,7 @@ unsafe fn duplicate_platformhandle( &mut target_handle, 0, FALSE, - winnt::DUPLICATE_CLOSE_SOURCE | winnt::DUPLICATE_SAME_ACCESS, + options, ); handleapi::CloseHandle(target); if ok == FALSE { diff --git a/third_party/rust/audioipc/src/lib.rs b/third_party/rust/audioipc/src/lib.rs index 4b877e324917..2798fa5c0176 100644 --- a/third_party/rust/audioipc/src/lib.rs +++ b/third_party/rust/audioipc/src/lib.rs @@ -50,9 +50,9 @@ pub use crate::messages::{ClientMessage, ServerMessage}; pub const SHM_AREA_SIZE: usize = 2 * 1024 * 1024; #[cfg(unix)] -use std::os::unix::io::{FromRawFd, IntoRawFd}; +use std::os::unix::io::IntoRawFd; #[cfg(windows)] -use std::os::windows::io::{FromRawHandle, IntoRawHandle}; +use std::os::windows::io::IntoRawHandle; use std::cell::RefCell; @@ -146,22 +146,22 @@ impl PlatformHandle { PlatformHandle::new(from.into_raw_fd(), true) } - #[cfg(windows)] - pub unsafe fn into_file(&self) -> std::fs::File { - std::fs::File::from_raw_handle(self.into_raw()) - } - - #[cfg(unix)] - pub unsafe fn into_file(&self) -> std::fs::File { - std::fs::File::from_raw_fd(self.into_raw()) - } - pub unsafe fn into_raw(&self) -> PlatformHandleType { let mut h = self.0.borrow_mut(); assert!(h.owned); h.owned = false; h.handle } + + pub unsafe fn as_raw(&self) -> PlatformHandleType { + self.0.borrow().handle + } + + #[cfg(windows)] + pub fn duplicate(h: PlatformHandleType) -> Result { + let dup = unsafe { platformhandle_passing::duplicate_platformhandle(h, None, false) }?; + Ok(PlatformHandle::new(dup, true)) + } } impl Drop for PlatformHandle { diff --git a/third_party/rust/audioipc/src/shm.rs b/third_party/rust/audioipc/src/shm.rs index 99012bfab405..a4f45911ffc0 100644 --- a/third_party/rust/audioipc/src/shm.rs +++ b/third_party/rust/audioipc/src/shm.rs @@ -4,263 +4,287 @@ // accompanying file LICENSE for details. use crate::errors::*; -use memmap::{Mmap, MmapMut, MmapOptions}; -use std::cell::UnsafeCell; -use std::convert::TryInto; -use std::env::temp_dir; -use std::fs::{remove_file, File, OpenOptions}; -use std::sync::{atomic, Arc}; +use crate::PlatformHandle; +use std::{convert::TryInto, ffi::c_void, slice}; -fn open_shm_file(id: &str) -> Result { - #[cfg(target_os = "linux")] - { - let id_cstring = std::ffi::CString::new(id).unwrap(); - unsafe { - let r = libc::syscall(libc::SYS_memfd_create, id_cstring.as_ptr(), 0); - if r >= 0 { - use std::os::unix::io::FromRawFd as _; - return Ok(File::from_raw_fd(r.try_into().unwrap())); +#[cfg(unix)] +pub use unix::SharedMem; +#[cfg(windows)] +pub use windows::SharedMem; + +#[derive(Copy, Clone)] +pub struct SharedMemView { + ptr: *mut c_void, + size: usize, +} + +unsafe impl Send for SharedMemView {} + +impl SharedMemView { + pub unsafe fn get_slice(&self, size: usize) -> Result<&[u8]> { + let map = slice::from_raw_parts(self.ptr as _, self.size); + if size <= self.size { + Ok(&map[..size]) + } else { + bail!("mmap size"); + } + } + + pub unsafe fn get_mut_slice(&mut self, size: usize) -> Result<&mut [u8]> { + let map = slice::from_raw_parts_mut(self.ptr as _, self.size); + if size <= self.size { + Ok(&mut map[..size]) + } else { + bail!("mmap size") + } + } +} + +#[cfg(unix)] +mod unix { + use super::*; + use memmap::{MmapMut, MmapOptions}; + use std::env::temp_dir; + use std::fs::{remove_file, File, OpenOptions}; + use std::os::unix::io::FromRawFd; + + fn open_shm_file(id: &str) -> Result { + #[cfg(target_os = "linux")] + { + let id_cstring = std::ffi::CString::new(id).unwrap(); + unsafe { + let r = libc::syscall(libc::SYS_memfd_create, id_cstring.as_ptr(), 0); + if r >= 0 { + use std::os::unix::io::FromRawFd as _; + return Ok(File::from_raw_fd(r.try_into().unwrap())); + } + } + + let mut path = std::path::PathBuf::from("/dev/shm"); + path.push(id); + + if let Ok(file) = OpenOptions::new() + .read(true) + .write(true) + .create_new(true) + .open(&path) + { + let _ = remove_file(&path); + return Ok(file); } } - let mut path = std::path::PathBuf::from("/dev/shm"); + let mut path = temp_dir(); path.push(id); - if let Ok(file) = OpenOptions::new() + let file = OpenOptions::new() .read(true) .write(true) .create_new(true) - .open(&path) + .open(&path)?; + + let _ = remove_file(&path); + Ok(file) + } + + fn handle_enospc(s: &str) -> Result<()> { + let err = std::io::Error::last_os_error(); + let errno = err.raw_os_error().unwrap_or(0); + assert_ne!(errno, 0); + debug!("allocate_file: {} failed errno={}", s, errno); + if errno == libc::ENOSPC { + return Err(err.into()); + } + Ok(()) + } + + fn allocate_file(file: &File, size: usize) -> Result<()> { + use std::os::unix::io::AsRawFd; + + // First, set the file size. This may create a sparse file on + // many systems, which can fail with SIGBUS when accessed via a + // mapping and the lazy backing allocation fails due to low disk + // space. To avoid this, try to force the entire file to be + // preallocated before mapping using OS-specific approaches below. + + file.set_len(size.try_into().unwrap())?; + + let fd = file.as_raw_fd(); + let size: libc::off_t = size.try_into().unwrap(); + + // Try Linux-specific fallocate. + #[cfg(target_os = "linux")] { - let _ = remove_file(&path); - return Ok(file); + if unsafe { libc::fallocate(fd, 0, 0, size) } == 0 { + return Ok(()); + } + handle_enospc("fallocate()")?; + } + + // Try macOS-specific fcntl. + #[cfg(target_os = "macos")] + { + let params = libc::fstore_t { + fst_flags: libc::F_ALLOCATEALL, + fst_posmode: libc::F_PEOFPOSMODE, + fst_offset: 0, + fst_length: size, + fst_bytesalloc: 0, + }; + if unsafe { libc::fcntl(fd, libc::F_PREALLOCATE, ¶ms) } == 0 { + return Ok(()); + } + handle_enospc("fcntl(F_PREALLOCATE)")?; + } + + // Fall back to portable version, where available. + #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly"))] + { + if unsafe { libc::posix_fallocate(fd, 0, size) } == 0 { + return Ok(()); + } + handle_enospc("posix_fallocate()")?; + } + + Ok(()) + } + + pub struct SharedMem { + _mmap: MmapMut, + view: SharedMemView, + } + + impl SharedMem { + pub fn new(id: &str, size: usize) -> Result<(SharedMem, PlatformHandle)> { + let file = open_shm_file(id)?; + allocate_file(&file, size)?; + let mut mmap = unsafe { MmapOptions::new().map_mut(&file)? }; + assert_eq!(mmap.len(), size); + let view = SharedMemView { + ptr: mmap.as_mut_ptr() as _, + size, + }; + let handle = PlatformHandle::from(file); + Ok((SharedMem { _mmap: mmap, view }, handle)) + } + + pub unsafe fn from(handle: &PlatformHandle, size: usize) -> Result { + let mut mmap = { + let file = File::from_raw_fd(handle.into_raw()); + MmapOptions::new().map_mut(&file)? + }; + assert_eq!(mmap.len(), size); + let view = SharedMemView { + ptr: mmap.as_mut_ptr() as _, + size, + }; + Ok(SharedMem { _mmap: mmap, view }) + } + + pub unsafe fn unsafe_view(&self) -> SharedMemView { + self.view + } + + pub unsafe fn get_slice(&self, size: usize) -> Result<&[u8]> { + self.view.get_slice(size) + } + + pub unsafe fn get_mut_slice(&mut self, size: usize) -> Result<&mut [u8]> { + self.view.get_mut_slice(size) } } - - let mut path = temp_dir(); - path.push(id); - - let file = OpenOptions::new() - .read(true) - .write(true) - .create_new(true) - .open(&path)?; - - let _ = remove_file(&path); - Ok(file) -} - -#[cfg(unix)] -fn handle_enospc(s: &str) -> Result<()> { - let err = std::io::Error::last_os_error(); - let errno = err.raw_os_error().unwrap_or(0); - assert_ne!(errno, 0); - debug!("allocate_file: {} failed errno={}", s, errno); - if errno == libc::ENOSPC { - return Err(err.into()); - } - Ok(()) -} - -#[cfg(unix)] -fn allocate_file(file: &File, size: usize) -> Result<()> { - use std::os::unix::io::AsRawFd; - - // First, set the file size. This may create a sparse file on - // many systems, which can fail with SIGBUS when accessed via a - // mapping and the lazy backing allocation fails due to low disk - // space. To avoid this, try to force the entire file to be - // preallocated before mapping using OS-specific approaches below. - - file.set_len(size.try_into().unwrap())?; - - let fd = file.as_raw_fd(); - let size: libc::off_t = size.try_into().unwrap(); - - // Try Linux-specific fallocate. - #[cfg(target_os = "linux")] - { - if unsafe { libc::fallocate(fd, 0, 0, size) } == 0 { - return Ok(()); - } - handle_enospc("fallocate()")?; - } - - // Try macOS-specific fcntl. - #[cfg(target_os = "macos")] - { - let params = libc::fstore_t { - fst_flags: libc::F_ALLOCATEALL, - fst_posmode: libc::F_PEOFPOSMODE, - fst_offset: 0, - fst_length: size, - fst_bytesalloc: 0, - }; - if unsafe { libc::fcntl(fd, libc::F_PREALLOCATE, ¶ms) } == 0 { - return Ok(()); - } - handle_enospc("fcntl(F_PREALLOCATE)")?; - } - - // Fall back to portable version, where available. - #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly"))] - { - if unsafe { libc::posix_fallocate(fd, 0, size) } == 0 { - return Ok(()); - } - handle_enospc("posix_fallocate()")?; - } - - Ok(()) } #[cfg(windows)] -fn allocate_file(file: &File, size: usize) -> Result<()> { - // CreateFileMapping will ensure the entire file is allocated - // before it's mapped in, so we simply set the size here. - file.set_len(size.try_into().unwrap())?; - Ok(()) -} +mod windows { + use super::*; + use std::ptr; + use winapi::{ + shared::{minwindef::DWORD, ntdef::HANDLE}, + um::{ + handleapi::CloseHandle, + memoryapi::{MapViewOfFile, UnmapViewOfFile, FILE_MAP_ALL_ACCESS}, + winbase::CreateFileMappingA, + winnt::PAGE_READWRITE, + }, + }; -pub struct SharedMemReader { - mmap: Mmap, -} + use crate::INVALID_HANDLE_VALUE; -impl SharedMemReader { - pub fn new(id: &str, size: usize) -> Result<(SharedMemReader, File)> { - let file = open_shm_file(id)?; - allocate_file(&file, size)?; - let mmap = unsafe { MmapOptions::new().map(&file)? }; - assert_eq!(mmap.len(), size); - Ok((SharedMemReader { mmap }, file)) + pub struct SharedMem { + handle: HANDLE, + view: SharedMemView, } - pub fn read(&self, buf: &mut [u8]) -> Result<()> { - if buf.is_empty() { - return Ok(()); + unsafe impl Send for SharedMem {} + + impl Drop for SharedMem { + fn drop(&mut self) { + unsafe { + let ok = UnmapViewOfFile(self.view.ptr); + assert_ne!(ok, 0); + if self.handle != INVALID_HANDLE_VALUE { + let ok = CloseHandle(self.handle); + assert_ne!(ok, 0); + } + } } - // TODO: Track how much is in the shm area. - if buf.len() <= self.mmap.len() { - atomic::fence(atomic::Ordering::Acquire); - let len = buf.len(); - buf.copy_from_slice(&self.mmap[..len]); - Ok(()) - } else { - bail!("mmap size"); + } + + impl SharedMem { + pub fn new(_id: &str, size: usize) -> Result<(SharedMem, PlatformHandle)> { + unsafe { + let handle = CreateFileMappingA( + INVALID_HANDLE_VALUE, + ptr::null_mut(), + PAGE_READWRITE, + (size as u64 >> 32).try_into().unwrap(), + (size as u64 & (DWORD::MAX as u64)).try_into().unwrap(), + ptr::null(), + ); + if handle.is_null() { + return Err(std::io::Error::last_os_error().into()); + } + + let ptr = MapViewOfFile(handle, FILE_MAP_ALL_ACCESS, 0, 0, size); + if ptr.is_null() { + return Err(std::io::Error::last_os_error().into()); + } + + let handle2 = PlatformHandle::duplicate(handle)?; + Ok(( + SharedMem { + handle, + view: SharedMemView { ptr, size }, + }, + handle2, + )) + } + } + + pub unsafe fn from(handle: &PlatformHandle, size: usize) -> Result { + let ptr = MapViewOfFile(handle.as_raw(), FILE_MAP_ALL_ACCESS, 0, 0, size); + if ptr.is_null() { + return Err(std::io::Error::last_os_error().into()); + } + Ok(SharedMem { + // A invalid `handle` means this is a non-owning `SharedMem`. See `Drop` impl. + // TODO: This can be made *owning* after further `PlatformHandle` ownership refactoring. + handle: INVALID_HANDLE_VALUE, + view: SharedMemView { ptr, size }, + }) + } + + pub unsafe fn unsafe_view(&self) -> SharedMemView { + self.view + } + + pub unsafe fn get_slice(&self, size: usize) -> Result<&[u8]> { + self.view.get_slice(size) + } + + pub unsafe fn get_mut_slice(&mut self, size: usize) -> Result<&mut [u8]> { + self.view.get_mut_slice(size) } } } - -pub struct SharedMemSlice { - mmap: Arc, -} - -impl SharedMemSlice { - pub fn from(file: &File, size: usize) -> Result { - let mmap = unsafe { MmapOptions::new().map(file)? }; - assert_eq!(mmap.len(), size); - let mmap = Arc::new(mmap); - Ok(SharedMemSlice { mmap }) - } - - pub fn get_slice(&self, size: usize) -> Result<&[u8]> { - if size == 0 { - return Ok(&[]); - } - // TODO: Track how much is in the shm area. - if size <= self.mmap.len() { - atomic::fence(atomic::Ordering::Acquire); - let buf = &self.mmap[..size]; - Ok(buf) - } else { - bail!("mmap size"); - } - } - - /// Clones the memory map. - /// - /// The underlying memory map is shared, and thus the caller must - /// ensure that the memory is not illegally aliased. - pub unsafe fn unsafe_clone(&self) -> Self { - SharedMemSlice { - mmap: self.mmap.clone(), - } - } -} - -unsafe impl Send for SharedMemSlice {} - -pub struct SharedMemWriter { - mmap: MmapMut, -} - -impl SharedMemWriter { - pub fn new(id: &str, size: usize) -> Result<(SharedMemWriter, File)> { - let file = open_shm_file(id)?; - allocate_file(&file, size)?; - let mmap = unsafe { MmapOptions::new().map_mut(&file)? }; - assert_eq!(mmap.len(), size); - Ok((SharedMemWriter { mmap }, file)) - } - - pub fn write(&mut self, buf: &[u8]) -> Result<()> { - if buf.is_empty() { - return Ok(()); - } - // TODO: Track how much is in the shm area. - if buf.len() <= self.mmap.len() { - self.mmap[..buf.len()].copy_from_slice(buf); - atomic::fence(atomic::Ordering::Release); - Ok(()) - } else { - bail!("mmap size"); - } - } -} - -pub struct SharedMemMutSlice { - mmap: Arc>, -} - -impl SharedMemMutSlice { - pub fn from(file: &File, size: usize) -> Result { - let mmap = unsafe { MmapOptions::new().map_mut(file)? }; - assert_eq!(mmap.len(), size); - let mmap = Arc::new(UnsafeCell::new(mmap)); - Ok(SharedMemMutSlice { mmap }) - } - - pub fn get_mut_slice(&mut self, size: usize) -> Result<&mut [u8]> { - if size == 0 { - return Ok(&mut []); - } - // TODO: Track how much is in the shm area. - if size <= self.inner().len() { - let buf = &mut self.inner_mut()[..size]; - atomic::fence(atomic::Ordering::Release); - Ok(buf) - } else { - bail!("mmap size"); - } - } - - /// Clones the memory map. - /// - /// The underlying memory map is shared, and thus the caller must - /// ensure that the memory is not illegally aliased. - pub unsafe fn unsafe_clone(&self) -> Self { - SharedMemMutSlice { - mmap: self.mmap.clone(), - } - } - - fn inner(&self) -> &MmapMut { - unsafe { &*self.mmap.get() } - } - - fn inner_mut(&mut self) -> &mut MmapMut { - unsafe { &mut *self.mmap.get() } - } -} - -unsafe impl Send for SharedMemMutSlice {} diff --git a/toolkit/library/rust/shared/Cargo.toml b/toolkit/library/rust/shared/Cargo.toml index 41e8f298dfd9..4094022d27e5 100644 --- a/toolkit/library/rust/shared/Cargo.toml +++ b/toolkit/library/rust/shared/Cargo.toml @@ -23,8 +23,8 @@ cubeb-coreaudio = { git = "https://github.com/mozilla/cubeb-coreaudio-rs", rev = cubeb-pulse = { git = "https://github.com/mozilla/cubeb-pulse-rs", rev="c87b50aebfa088c1ad30c74819d4e9829f88b2e3", optional = true, features=["pulse-dlopen"] } cubeb-sys = { version = "0.9", optional = true, features=["gecko-in-tree"] } encoding_glue = { path = "../../../../intl/encoding_glue" } -audioipc-client = { git = "https://github.com/mozilla/audioipc-2", rev = "ca5abc4bb056e18e4b8e5b2bb5ed9a8a21443ee7", optional = true } -audioipc-server = { git = "https://github.com/mozilla/audioipc-2", rev = "ca5abc4bb056e18e4b8e5b2bb5ed9a8a21443ee7", optional = true } +audioipc-client = { git = "https://github.com/mozilla/audioipc-2", rev = "8fb5ff19fba7b09e8e66598122421e68a5c573ac", optional = true } +audioipc-server = { git = "https://github.com/mozilla/audioipc-2", rev = "8fb5ff19fba7b09e8e66598122421e68a5c573ac", optional = true } authenticator = "0.3.1" gkrust_utils = { path = "../../../../xpcom/rust/gkrust_utils" } gecko_logger = { path = "../../../../xpcom/rust/gecko_logger" }