diff --git a/Cargo.lock b/Cargo.lock index 20cae72db4edcd2d27fdfe03bcaced18f974761f..c8ddeebb26a9ec2787b835996451a6fac11d659d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7390,6 +7390,7 @@ dependencies = [ "parking", "parking_lot", "pathfinder_geometry", + "pin-project", "postage", "pretty_assertions", "profiling", diff --git a/crates/gpui/Cargo.toml b/crates/gpui/Cargo.toml index 20a3a4553e363b6b04458927178c4dc5d612101c..b03a5272260c6d92efc89db971d39ae93b12982b 100644 --- a/crates/gpui/Cargo.toml +++ b/crates/gpui/Cargo.toml @@ -135,6 +135,7 @@ waker-fn = "1.2.0" lyon = "1.0" workspace-hack.workspace = true libc.workspace = true +pin-project = "1.1.10" [target.'cfg(target_os = "macos")'.dependencies] block = "0.1" diff --git a/crates/gpui/src/app/entity_map.rs b/crates/gpui/src/app/entity_map.rs index ea52b46d9fce958f8cb6e878581fb988c146c43b..bea98cb06a5f80fc8141a52bc47f48e8734b40c9 100644 --- a/crates/gpui/src/app/entity_map.rs +++ b/crates/gpui/src/app/entity_map.rs @@ -378,11 +378,9 @@ pub struct Entity { #[deref] #[deref_mut] pub(crate) any_entity: AnyEntity, - pub(crate) entity_type: PhantomData, + pub(crate) entity_type: PhantomData T>, } -unsafe impl Send for Entity {} -unsafe impl Sync for Entity {} impl Sealed for Entity {} impl Entity { @@ -657,7 +655,7 @@ pub struct WeakEntity { #[deref] #[deref_mut] any_entity: AnyWeakEntity, - entity_type: PhantomData, + entity_type: PhantomData T>, } impl std::fmt::Debug for WeakEntity { @@ -669,9 +667,6 @@ impl std::fmt::Debug for WeakEntity { } } -unsafe impl Send for WeakEntity {} -unsafe impl Sync for WeakEntity {} - impl Clone for WeakEntity { fn clone(&self) -> Self { Self { diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index b3d342b09bf1dceb27413d3ec24fbcc0d2f541e9..2e24c3ba195f9fc8f72c43cd3da09778e8b2b3e9 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -888,7 +888,9 @@ impl VisualTestContext { // safety: on_quit will be called after the test has finished. // the executor will ensure that all tasks related to the test have stopped. // so there is no way for cx to be accessed after on_quit is called. - let cx = Box::leak(unsafe { Box::from_raw(ptr) }); + // todo: This is unsound under stacked borrows (also tree borrows probably?) + // the mutable reference invalidates `ptr` which is later used in the closure + let cx = unsafe { &mut *ptr }; cx.on_quit(move || unsafe { drop(Box::from_raw(ptr)); }); diff --git a/crates/gpui/src/arena.rs b/crates/gpui/src/arena.rs index a0d0c23987472de46d5b23129adb5a4ec8ee00cb..9898c8056ab0240abd32ee34992dbe96f8ebab57 100644 --- a/crates/gpui/src/arena.rs +++ b/crates/gpui/src/arena.rs @@ -15,9 +15,7 @@ struct ArenaElement { impl Drop for ArenaElement { #[inline(always)] fn drop(&mut self) { - unsafe { - (self.drop)(self.value); - } + unsafe { (self.drop)(self.value) }; } } @@ -40,33 +38,29 @@ impl Drop for Chunk { impl Chunk { fn new(chunk_size: NonZeroUsize) -> Self { - unsafe { - // this only fails if chunk_size is unreasonably huge - let layout = alloc::Layout::from_size_align(chunk_size.get(), 1).unwrap(); - let start = alloc::alloc(layout); - if start.is_null() { - handle_alloc_error(layout); - } - let end = start.add(chunk_size.get()); - Self { - start, - end, - offset: start, - } + // this only fails if chunk_size is unreasonably huge + let layout = alloc::Layout::from_size_align(chunk_size.get(), 1).unwrap(); + let start = unsafe { alloc::alloc(layout) }; + if start.is_null() { + handle_alloc_error(layout); + } + let end = unsafe { start.add(chunk_size.get()) }; + Self { + start, + end, + offset: start, } } fn allocate(&mut self, layout: alloc::Layout) -> Option> { - unsafe { - let aligned = self.offset.add(self.offset.align_offset(layout.align())); - let next = aligned.add(layout.size()); - - if next <= self.end { - self.offset = next; - NonNull::new(aligned) - } else { - None - } + let aligned = unsafe { self.offset.add(self.offset.align_offset(layout.align())) }; + let next = unsafe { aligned.add(layout.size()) }; + + if next <= self.end { + self.offset = next; + NonNull::new(aligned) + } else { + None } } @@ -122,54 +116,48 @@ impl Arena { where F: FnOnce() -> T, { - unsafe { - ptr::write(ptr, f()); - } + unsafe { ptr::write(ptr, f()) }; } unsafe fn drop(ptr: *mut u8) { - unsafe { - std::ptr::drop_in_place(ptr.cast::()); - } + unsafe { std::ptr::drop_in_place(ptr.cast::()) }; } - unsafe { - let layout = alloc::Layout::new::(); - let mut current_chunk = &mut self.chunks[self.current_chunk_index]; - let ptr = if let Some(ptr) = current_chunk.allocate(layout) { + let layout = alloc::Layout::new::(); + let mut current_chunk = &mut self.chunks[self.current_chunk_index]; + let ptr = if let Some(ptr) = current_chunk.allocate(layout) { + ptr.as_ptr() + } else { + self.current_chunk_index += 1; + if self.current_chunk_index >= self.chunks.len() { + self.chunks.push(Chunk::new(self.chunk_size)); + assert_eq!(self.current_chunk_index, self.chunks.len() - 1); + log::trace!( + "increased element arena capacity to {}kb", + self.capacity() / 1024, + ); + } + current_chunk = &mut self.chunks[self.current_chunk_index]; + if let Some(ptr) = current_chunk.allocate(layout) { ptr.as_ptr() } else { - self.current_chunk_index += 1; - if self.current_chunk_index >= self.chunks.len() { - self.chunks.push(Chunk::new(self.chunk_size)); - assert_eq!(self.current_chunk_index, self.chunks.len() - 1); - log::trace!( - "increased element arena capacity to {}kb", - self.capacity() / 1024, - ); - } - current_chunk = &mut self.chunks[self.current_chunk_index]; - if let Some(ptr) = current_chunk.allocate(layout) { - ptr.as_ptr() - } else { - panic!( - "Arena chunk_size of {} is too small to allocate {} bytes", - self.chunk_size, - layout.size() - ); - } - }; - - inner_writer(ptr.cast(), f); - self.elements.push(ArenaElement { - value: ptr, - drop: drop::, - }); - - ArenaBox { - ptr: ptr.cast(), - valid: self.valid.clone(), + panic!( + "Arena chunk_size of {} is too small to allocate {} bytes", + self.chunk_size, + layout.size() + ); } + }; + + unsafe { inner_writer(ptr.cast(), f) }; + self.elements.push(ArenaElement { + value: ptr, + drop: drop::, + }); + + ArenaBox { + ptr: ptr.cast(), + valid: self.valid.clone(), } } } diff --git a/crates/gpui/src/executor.rs b/crates/gpui/src/executor.rs index 3fd1c8580deee1b98c83bef7a39f1325a7a5ddc8..ab5cafcbac55b40b2e136f28a08f2245ae27e4f1 100644 --- a/crates/gpui/src/executor.rs +++ b/crates/gpui/src/executor.rs @@ -525,9 +525,7 @@ where "local task dropped by a thread that didn't spawn it. Task spawned at {}", self.location ); - unsafe { - ManuallyDrop::drop(&mut self.inner); - } + unsafe { ManuallyDrop::drop(&mut self.inner) }; } } diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 39ba72705136721866638940cdefbacc2f7b1b29..6847a34a843395a18531bcf3114d1dfe24f7bf29 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -348,8 +348,6 @@ impl Debug for DisplayId { } } -unsafe impl Send for DisplayId {} - /// Which part of the window to resize #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ResizeEdge { diff --git a/crates/gpui/src/taffy.rs b/crates/gpui/src/taffy.rs index bc35dcec1eb126931cd71a6f3a17ce05054e8dbe..29b4ce644f7b6e4221b72ac8dacc43b558b3eb8e 100644 --- a/crates/gpui/src/taffy.rs +++ b/crates/gpui/src/taffy.rs @@ -69,9 +69,7 @@ impl TaffyLayoutEngine { } else { self.taffy // This is safe because LayoutId is repr(transparent) to taffy::tree::NodeId. - .new_with_children(taffy_style, unsafe { - std::mem::transmute::<&[LayoutId], &[taffy::NodeId]>(children) - }) + .new_with_children(taffy_style, LayoutId::to_taffy_slice(children)) .expect(EXPECT_MESSAGE) .into() } @@ -265,6 +263,13 @@ impl TaffyLayoutEngine { #[repr(transparent)] pub struct LayoutId(NodeId); +impl LayoutId { + fn to_taffy_slice(node_ids: &[Self]) -> &[taffy::NodeId] { + // SAFETY: LayoutId is repr(transparent) to taffy::tree::NodeId. + unsafe { std::mem::transmute::<&[LayoutId], &[taffy::NodeId]>(node_ids) } + } +} + impl std::hash::Hash for LayoutId { fn hash(&self, state: &mut H) { u64::from(self.0).hash(state); diff --git a/crates/gpui/src/util.rs b/crates/gpui/src/util.rs index badb68008216400464e997f3252e9467edb234c6..92c86810c5e30c4c1bc614788b0f16f4966f3b4c 100644 --- a/crates/gpui/src/util.rs +++ b/crates/gpui/src/util.rs @@ -83,8 +83,11 @@ impl FutureExt for T { } } +#[pin_project::pin_project] pub struct WithTimeout { + #[pin] future: T, + #[pin] timer: Task<()>, } @@ -97,15 +100,11 @@ impl Future for WithTimeout { type Output = Result; fn poll(self: Pin<&mut Self>, cx: &mut task::Context) -> task::Poll { - // SAFETY: the fields of Timeout are private and we never move the future ourselves - // And its already pinned since we are being polled (all futures need to be pinned to be polled) - let this = unsafe { &raw mut *self.get_unchecked_mut() }; - let future = unsafe { Pin::new_unchecked(&mut (*this).future) }; - let timer = unsafe { Pin::new_unchecked(&mut (*this).timer) }; + let this = self.project(); - if let task::Poll::Ready(output) = future.poll(cx) { + if let task::Poll::Ready(output) = this.future.poll(cx) { task::Poll::Ready(Ok(output)) - } else if timer.poll(cx).is_ready() { + } else if this.timer.poll(cx).is_ready() { task::Poll::Ready(Err(Timeout)) } else { task::Poll::Pending diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index caf78fe407ea0a61a88efd9462be5fce005dedbf..5534a4a8ddf98c2bf5d26637b7af3a499dd63ae1 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -4650,7 +4650,7 @@ pub struct WindowHandle { #[deref] #[deref_mut] pub(crate) any_handle: AnyWindowHandle, - state_type: PhantomData, + state_type: PhantomData V>, } impl Debug for WindowHandle { @@ -4786,9 +4786,6 @@ impl From> for AnyWindowHandle { } } -unsafe impl Send for WindowHandle {} -unsafe impl Sync for WindowHandle {} - /// A handle to a window with any root view type, which can be downcast to a window with a specific root view type. #[derive(Copy, Clone, PartialEq, Eq, Hash)] pub struct AnyWindowHandle {