diff --git a/src/array_buffer.rs b/src/array_buffer.rs index c6b3724b..28a11961 100644 --- a/src/array_buffer.rs +++ b/src/array_buffer.rs @@ -7,7 +7,6 @@ use std::ptr::null_mut; use std::slice; use crate::support::long; -use crate::support::Delete; use crate::support::Opaque; use crate::support::Shared; use crate::support::SharedRef; @@ -137,8 +136,8 @@ fn test_default_allocator() { new_default_allocator(); } -impl Delete for Allocator { - fn delete(&mut self) { +impl Drop for Allocator { + fn drop(&mut self) { unsafe { v8__ArrayBuffer__Allocator__DELETE(self) }; } } @@ -212,8 +211,8 @@ impl DerefMut for BackingStore { } } -impl Delete for BackingStore { - fn delete(&mut self) { +impl Drop for BackingStore { + fn drop(&mut self) { unsafe { v8__BackingStore__DELETE(self) }; } } diff --git a/src/inspector.rs b/src/inspector.rs index d77110e3..00e829e5 100644 --- a/src/inspector.rs +++ b/src/inspector.rs @@ -16,7 +16,6 @@ use crate::scope_traits::InIsolate; use crate::support::int; use crate::support::CxxVTable; -use crate::support::Delete; use crate::support::FieldOffset; use crate::support::Opaque; use crate::support::RustVTable; @@ -583,8 +582,8 @@ impl V8InspectorSession { } } -impl Delete for V8InspectorSession { - fn delete(&mut self) { +impl Drop for V8InspectorSession { + fn drop(&mut self) { unsafe { v8_inspector__V8InspectorSession__DELETE(self) }; } } @@ -615,8 +614,8 @@ impl StringBuffer { } } -impl Delete for StringBuffer { - fn delete(&mut self) { +impl Drop for StringBuffer { + fn drop(&mut self) { unsafe { v8_inspector__StringBuffer__DELETE(self) } } } @@ -891,8 +890,8 @@ impl V8Inspector { } } -impl Delete for V8Inspector { - fn delete(&mut self) { +impl Drop for V8Inspector { + fn drop(&mut self) { unsafe { v8_inspector__V8Inspector__DELETE(self) }; } } diff --git a/src/isolate.rs b/src/isolate.rs index 52135800..07153024 100644 --- a/src/isolate.rs +++ b/src/isolate.rs @@ -3,7 +3,6 @@ use crate::array_buffer::Allocator; use crate::external_references::ExternalReferences; use crate::promise::PromiseRejectMessage; use crate::support::intptr_t; -use crate::support::Delete; use crate::support::Opaque; use crate::support::SharedRef; use crate::support::UniqueRef; @@ -575,8 +574,8 @@ impl CreateParams { } } -impl Delete for CreateParams { - fn delete(&mut self) { +impl Drop for CreateParams { + fn drop(&mut self) { unsafe { v8__Isolate__CreateParams__DELETE(self) } } } diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 1f87f944..4fa1be47 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -2,7 +2,6 @@ pub mod task; pub use task::{Task, TaskBase, TaskImpl}; -use crate::support::Delete; use crate::support::Opaque; use crate::support::UniquePtr; use crate::Isolate; @@ -22,8 +21,8 @@ pub fn new_default_platform() -> UniquePtr { #[repr(C)] pub struct Platform(Opaque); -impl Delete for Platform { - fn delete(&mut self) { +impl Drop for Platform { + fn drop(&mut self) { unsafe { v8__Platform__DELETE(self) } } } diff --git a/src/platform/task.rs b/src/platform/task.rs index 9e6d6bf8..bc1b3cb6 100644 --- a/src/platform/task.rs +++ b/src/platform/task.rs @@ -1,12 +1,12 @@ use std::mem::drop; use std::mem::forget; +use std::mem::ManuallyDrop; use crate::support::CxxVTable; -use crate::support::Delete; use crate::support::FieldOffset; use crate::support::Opaque; use crate::support::RustVTable; -use crate::support::UniquePtr; +use crate::support::UniqueRef; // class Task { // public: @@ -41,8 +41,8 @@ impl Task { } } -impl Delete for Task { - fn delete(&mut self) { +impl Drop for Task { + fn drop(&mut self) { unsafe { v8__Task__DELETE(self) } } } @@ -52,13 +52,13 @@ pub trait AsTask { fn as_task_mut(&mut self) -> &mut Task; // TODO: this should be a trait in itself. - fn into_unique_ptr(mut self: Box) -> UniquePtr + fn into_unique_ref(mut self: Box) -> UniqueRef where Self: 'static, { let task = self.as_task_mut() as *mut Task; forget(self); - unsafe { UniquePtr::from_raw(task) } + unsafe { UniqueRef::from_raw(task) } } } @@ -90,23 +90,23 @@ pub trait TaskImpl: AsTask { } pub struct TaskBase { - cxx_base: Task, + cxx_base: ManuallyDrop, offset_within_embedder: FieldOffset, rust_vtable: RustVTable<&'static dyn TaskImpl>, } impl TaskBase { - fn construct_cxx_base() -> Task { + fn construct_cxx_base() -> ManuallyDrop { unsafe { let mut buf = std::mem::MaybeUninit::::uninit(); v8__Task__BASE__CONSTRUCT(&mut buf); - buf.assume_init() + ManuallyDrop::new(buf.assume_init()) } } fn get_cxx_base_offset() -> FieldOffset { let buf = std::mem::MaybeUninit::::uninit(); - FieldOffset::from_ptrs(buf.as_ptr(), unsafe { &(*buf.as_ptr()).cxx_base }) + FieldOffset::from_ptrs(buf.as_ptr(), unsafe { &*(*buf.as_ptr()).cxx_base }) } fn get_offset_within_embedder() -> FieldOffset @@ -208,17 +208,21 @@ mod tests { } #[test] - fn test_task() { - { - TestTask::new().run(); - } + fn test_task_1() { + let mut task = TestTask::new(); + task.run(); + drop(task); assert_eq!(RUN_COUNT.swap(0, SeqCst), 1); assert_eq!(DROP_COUNT.swap(0, SeqCst), 1); + } - { - Box::new(TestTask::new()).into_unique_ptr(); - } - assert_eq!(RUN_COUNT.swap(0, SeqCst), 0); + #[test] + fn test_task_2() { + let mut task = Box::new(TestTask::new()).into_unique_ref(); + task.run(); + task.run(); + drop(task); + assert_eq!(RUN_COUNT.swap(0, SeqCst), 2); assert_eq!(DROP_COUNT.swap(0, SeqCst), 1); } } diff --git a/src/support.rs b/src/support.rs index 1fdb4d69..682e5a46 100644 --- a/src/support.rs +++ b/src/support.rs @@ -1,10 +1,12 @@ use std::cell::UnsafeCell; use std::marker::PhantomData; +use std::mem::align_of; use std::mem::forget; +use std::mem::needs_drop; use std::mem::size_of; -use std::mem::transmute; use std::ops::Deref; use std::ops::DerefMut; +use std::ptr::drop_in_place; use std::ptr::null_mut; use std::ptr::NonNull; @@ -19,28 +21,18 @@ pub use std::os::raw::c_long as long; pub type Opaque = [u8; 0]; -pub trait Delete -where - Self: Sized, -{ - fn delete(&mut self) -> (); -} - /// Pointer to object allocated on the C++ heap. The pointer may be null. #[repr(transparent)] -pub struct UniquePtr(Option>) -where - T: Delete; +pub struct UniquePtr(Option>); -impl UniquePtr -where - T: Delete, -{ +impl UniquePtr { pub fn null() -> Self { + assert_unique_type_compatible::(); Self(None) } pub unsafe fn from_raw(ptr: *mut T) -> Self { + assert_unique_type_compatible::(); Self(UniqueRef::try_from_raw(ptr)) } @@ -54,37 +46,22 @@ where pub fn unwrap(self) -> UniqueRef { self.0.unwrap() } - - unsafe fn _static_assert_has_pointer_repr() { - let dummy: fn() -> Self = || unimplemented!(); - let _ptr: *mut T = transmute(dummy()); - let _ref: &mut T = transmute(dummy()); - } } -impl From> for UniquePtr -where - T: Delete, -{ +impl From> for UniquePtr { fn from(unique_ref: UniqueRef) -> Self { Self(Some(unique_ref)) } } -impl Deref for UniquePtr -where - T: Delete, -{ +impl Deref for UniquePtr { type Target = Option>; fn deref(&self) -> &Self::Target { &self.0 } } -impl DerefMut for UniquePtr -where - T: Delete, -{ +impl DerefMut for UniquePtr { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } @@ -92,19 +69,12 @@ where /// Pointer to object allocated on the C++ heap. The pointer may not be null. #[repr(transparent)] -pub struct UniqueRef(NonNull) -where - T: Delete; +pub struct UniqueRef(NonNull); -impl UniqueRef -where - T: Delete, -{ - pub fn make_shared(self) -> SharedRef - where - T: Shared, - { - self.into() +impl UniqueRef { + unsafe fn try_from_raw(ptr: *mut T) -> Option { + assert_unique_type_compatible::(); + NonNull::new(ptr).map(Self) } pub unsafe fn from_raw(ptr: *mut T) -> Self { @@ -117,48 +87,47 @@ where ptr } - unsafe fn try_from_raw(ptr: *mut T) -> Option { - NonNull::new(ptr).map(Self) - } - - unsafe fn _static_assert_has_pointer_repr() { - let dummy: fn() -> Self = || unimplemented!(); - let _ptr: *mut T = transmute(dummy()); - let _ref: &mut T = transmute(dummy()); + pub fn make_shared(self) -> SharedRef + where + T: Shared, + { + self.into() } } -impl Deref for UniqueRef -where - T: Delete, -{ +impl Deref for UniqueRef { type Target = T; fn deref(&self) -> &Self::Target { unsafe { self.0.as_ref() } } } -impl DerefMut for UniqueRef -where - T: Delete, -{ +impl DerefMut for UniqueRef { fn deref_mut(&mut self) -> &mut Self::Target { unsafe { self.0.as_mut() } } } -impl Drop for UniqueRef -where - T: Delete, -{ +impl Drop for UniqueRef { fn drop(&mut self) { - Delete::delete(&mut **self) + unsafe { drop_in_place(self.0.as_ptr()) } } } +fn assert_unique_type_compatible() { + // Assert that `U` (a `UniqueRef` or `UniquePtr`) has the same memory layout + // as a pointer to `T`. + assert_eq!(size_of::(), size_of::<*mut T>()); + assert_eq!(align_of::(), align_of::<*mut T>()); + + // Assert that `T` (probably) implements `Drop`. If it doesn't, a regular + // reference should be used instead of UniquePtr/UniqueRef. + assert!(needs_drop::()); +} + pub trait Shared where - Self: Delete + 'static, + Self: Sized, { fn clone(shared_ptr: *const SharedRef) -> SharedRef; fn from_unique(unique: UniqueRef) -> SharedRef; @@ -196,7 +165,7 @@ where impl From> for SharedRef where - T: Delete + Shared, + T: Shared, { fn from(unique: UniqueRef) -> Self { ::from_unique(unique)