diff --git a/base/src/sys/windows/named_pipes.rs b/base/src/sys/windows/named_pipes.rs index eb8e1f304d..63d397198c 100644 --- a/base/src/sys/windows/named_pipes.rs +++ b/base/src/sys/windows/named_pipes.rs @@ -89,12 +89,22 @@ pub struct PipeConnection { blocking_mode: BlockingMode, } +/// `OVERLAPPED` is allocated on the heap because it must not move while performing I/O operations. +/// +/// Defined as a separate type so that we can mark it as `Send` and `Sync`. +pub struct BoxedOverlapped(pub Box); + +// SAFETY: `OVERLAPPED` is not automatically `Send` because it contains a `HANDLE`, which is a raw +// pointer, but `HANDLE`s are safe to move between threads and thus so is `OVERLAPPED`. +unsafe impl Send for BoxedOverlapped {} + +// SAFETY: See the argument for `Send` above. `HANDLE`s are also safe to share between threads. +unsafe impl Sync for BoxedOverlapped {} + /// Wraps the OVERLAPPED structure. Also keeps track of whether OVERLAPPED is being used by a /// Readfile or WriteFile operation and holds onto the event object so it doesn't get dropped. pub struct OverlappedWrapper { - // Allocated on the heap so that the OVERLAPPED struct doesn't move when performing I/O - // operations. - overlapped: Box, + overlapped: BoxedOverlapped, // This field prevents the event handle from being dropped too early and allows callers to // be notified when a read or write overlapped operation has completed. h_event: Option, @@ -130,17 +140,13 @@ impl OverlappedWrapper { }; Ok(OverlappedWrapper { - overlapped: Box::new(overlapped), + overlapped: BoxedOverlapped(Box::new(overlapped)), h_event, in_use: false, }) } } -// SAFETY: -// Safe because all of the contained fields may be safely sent to another thread. -unsafe impl Send for OverlappedWrapper {} - pub trait WriteOverlapped { /// Perform an overlapped write operation with the specified buffer and overlapped wrapper. /// If successful, the write operation will complete asynchronously, and @@ -564,7 +570,7 @@ impl PipeConnection { &self.handle, self.blocking_mode, buf, - Some(&mut overlapped_wrapper.overlapped), + Some(&mut overlapped_wrapper.overlapped.0), )?; Ok(()) } @@ -727,7 +733,7 @@ impl PipeConnection { PipeConnection::write_internal( &self.handle, buf, - Some(&mut overlapped_wrapper.overlapped), + Some(&mut overlapped_wrapper.overlapped.0), )?; Ok(()) } @@ -850,7 +856,7 @@ impl PipeConnection { self.as_raw_descriptor(), // Note: The overlapped structure is only used if the pipe was opened in // OVERLAPPED mode, but is necessary in that case. - &mut *overlapped_wrapper.overlapped, + &mut *overlapped_wrapper.overlapped.0, ); if success_flag == 0 { return match GetLastError() { @@ -934,7 +940,7 @@ impl PipeConnection { fail_if_zero!(unsafe { GetOverlappedResult( self.handle.as_raw_descriptor(), - &mut *overlapped_wrapper.overlapped, + &mut *overlapped_wrapper.overlapped.0, &mut size_transferred, if wait { TRUE } else { FALSE }, ) diff --git a/cros_async/src/sys/windows/handle_executor.rs b/cros_async/src/sys/windows/handle_executor.rs index fae3771af0..17f8611600 100644 --- a/cros_async/src/sys/windows/handle_executor.rs +++ b/cros_async/src/sys/windows/handle_executor.rs @@ -12,6 +12,7 @@ use std::sync::Arc; use std::sync::Weak; use std::task::Waker; +use base::named_pipes::BoxedOverlapped; use base::warn; use base::AsRawDescriptor; use base::Error as SysError; @@ -234,9 +235,9 @@ impl RegisteredOverlappedSource { /// in the IO call. pub fn register_overlapped_operation( &self, - overlapped: OVERLAPPED, + offset: Option, ) -> Result { - OverlappedOperation::new(overlapped, self.ex.clone()) + OverlappedOperation::new(offset, self.ex.clone()) } } @@ -256,15 +257,15 @@ impl WeakWake for HandleReactor { /// ensure the waker has been registered. (If the executor polls the IOCP before the waker /// is registered, the future will stall.) pub(crate) struct OverlappedOperation { - overlapped: Pin>, + overlapped: BoxedOverlapped, ex: Weak>, completed: bool, } impl OverlappedOperation { - fn new(overlapped: OVERLAPPED, ex: Weak>) -> Result { + fn new(offset: Option, ex: Weak>) -> Result { let ret = Self { - overlapped: Box::pin(overlapped), + overlapped: BoxedOverlapped(Box::new(base::create_overlapped(offset, None))), ex, completed: false, }; @@ -285,12 +286,12 @@ impl OverlappedOperation { /// when making the overlapped IO call or the executor will not be able to wake the right /// future. pub fn get_overlapped(&mut self) -> &mut OVERLAPPED { - &mut self.overlapped + &mut self.overlapped.0 } #[inline] pub fn get_token(&self) -> WakerToken { - WakerToken((&*self.overlapped) as *const _ as usize) + WakerToken((&*self.overlapped.0) as *const _ as usize) } } diff --git a/cros_async/src/sys/windows/overlapped_source.rs b/cros_async/src/sys/windows/overlapped_source.rs index 802beb9470..629a3662f7 100644 --- a/cros_async/src/sys/windows/overlapped_source.rs +++ b/cros_async/src/sys/windows/overlapped_source.rs @@ -10,7 +10,6 @@ use std::io::Write; use std::mem::ManuallyDrop; use std::sync::Arc; -use base::create_overlapped; use base::error; use base::AsRawDescriptor; use base::Descriptor; @@ -146,8 +145,7 @@ impl OverlappedSource { io::Error::new(io::ErrorKind::InvalidInput, "seek on non-seekable handle"), ))); } - let overlapped = create_overlapped(file_offset, None); - let mut overlapped_op = self.reg_source.register_overlapped_operation(overlapped)?; + let mut overlapped_op = self.reg_source.register_overlapped_operation(file_offset)?; // SAFETY: // Safe because we pass a pointer to a valid vec and that same vector's length. @@ -188,8 +186,7 @@ impl OverlappedSource { }; for region in mem_offsets.into_iter() { - let overlapped = create_overlapped(offset, None); - let mut overlapped_op = self.reg_source.register_overlapped_operation(overlapped)?; + let mut overlapped_op = self.reg_source.register_overlapped_operation(offset)?; let slice = mem.get_volatile_slice(region).map_err(|e| { AsyncError::OverlappedSource(Error::BackingMemoryVolatileSliceFetchFailed(e)) @@ -236,8 +233,7 @@ impl OverlappedSource { io::Error::new(io::ErrorKind::InvalidInput, "seek on non-seekable handle"), ))); } - let overlapped = create_overlapped(file_offset, None); - let mut overlapped_op = self.reg_source.register_overlapped_operation(overlapped)?; + let mut overlapped_op = self.reg_source.register_overlapped_operation(file_offset)?; // SAFETY: // Safe because we pass a pointer to a valid vec and that same vector's length. @@ -279,8 +275,7 @@ impl OverlappedSource { }; for region in mem_offsets.into_iter() { - let overlapped = create_overlapped(offset, None); - let mut overlapped_op = self.reg_source.register_overlapped_operation(overlapped)?; + let mut overlapped_op = self.reg_source.register_overlapped_operation(offset)?; let slice = mem.get_volatile_slice(region).map_err(|e| { AsyncError::OverlappedSource(Error::BackingMemoryVolatileSliceFetchFailed(e))