From c2471a743787a41964ef5a9987a2732893f3a94f Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Wed, 20 Apr 2022 00:20:12 -0400 Subject: [PATCH 1/2] Convert UnsafeWeakPointer from usize to *mut T --- Cargo.toml | 2 +- src/common/unsafe_weak_pointer.rs | 32 ++++++++++++------------------- src/sync/housekeeper.rs | 12 ++++++------ src/sync/invalidator.rs | 4 ++-- 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2c777810..0c8a2087 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ unstable-debug-counters = ["future"] [dependencies] crossbeam-channel = "0.5.4" -crossbeam-epoch = "0.8.2" +crossbeam-epoch = "0.9" crossbeam-utils = "0.8" num_cpus = "1.13" once_cell = "1.7" diff --git a/src/common/unsafe_weak_pointer.rs b/src/common/unsafe_weak_pointer.rs index b42b2fab..e8f5341d 100644 --- a/src/common/unsafe_weak_pointer.rs +++ b/src/common/unsafe_weak_pointer.rs @@ -3,39 +3,31 @@ use std::sync::{Arc, Weak}; /// WARNING: Do not use this struct unless you are absolutely sure what you are /// doing. Using this struct is unsafe and may cause memory related crashes and/or /// security vulnerabilities. -/// -/// This struct exists with the sole purpose of avoiding compile errors relevant to -/// the thread pool usages. The thread pool requires that the generic parameters on -/// the `Cache` and `Inner` structs to have trait bounds `Send`, `Sync` and -/// `'static`. This will be unacceptable for many cache usages. -/// -/// This struct avoids the trait bounds by transmuting a pointer between -/// `std::sync::Weak>` and `usize`. -/// -/// If you know a better solution than this, we would love te hear it. -pub(crate) struct UnsafeWeakPointer { +pub(crate) struct UnsafeWeakPointer { // This is a std::sync::Weak pointer to Inner. - raw_ptr: usize, + raw_ptr: *mut T, } -impl UnsafeWeakPointer { - pub(crate) fn from_weak_arc(p: Weak) -> Self { +unsafe impl Send for UnsafeWeakPointer {} + +impl UnsafeWeakPointer { + pub(crate) fn from_weak_arc(p: Weak) -> Self { Self { - raw_ptr: unsafe { std::mem::transmute(p) }, + raw_ptr: p.into_raw() as *mut T, } } - pub(crate) unsafe fn as_weak_arc(&self) -> Weak { - std::mem::transmute(self.raw_ptr) + pub(crate) unsafe fn as_weak_arc(&self) -> Weak { + Weak::from_raw(self.raw_ptr.cast()) } - pub(crate) fn forget_arc(p: Arc) { + pub(crate) fn forget_arc(p: Arc) { // Downgrade the Arc to Weak, then forget. let weak = Arc::downgrade(&p); std::mem::forget(weak); } - pub(crate) fn forget_weak_arc(p: Weak) { + pub(crate) fn forget_weak_arc(p: Weak) { std::mem::forget(p); } } @@ -46,7 +38,7 @@ impl UnsafeWeakPointer { /// /// When you want to drop the Weak pointer, ensure that you drop it only once for the /// same `raw_ptr` across clones. -impl Clone for UnsafeWeakPointer { +impl Clone for UnsafeWeakPointer { fn clone(&self) -> Self { Self { raw_ptr: self.raw_ptr, diff --git a/src/sync/housekeeper.rs b/src/sync/housekeeper.rs index 3babc063..62705014 100644 --- a/src/sync/housekeeper.rs +++ b/src/sync/housekeeper.rs @@ -39,7 +39,7 @@ pub(crate) trait InnerSync { } pub(crate) struct Housekeeper { - inner: Arc>, + inner: Arc>>, thread_pool: Arc, is_shutting_down: Arc, periodical_sync_job: Mutex>, @@ -73,12 +73,12 @@ impl Drop for Housekeeper { // All sync jobs should have been finished by now. Clean other stuff up. ThreadPoolRegistry::release_pool(&self.thread_pool); - std::mem::drop(unsafe { self.inner.lock().as_weak_arc::() }); + std::mem::drop(unsafe { self.inner.lock().as_weak_arc() }); } } // functions/methods used by Cache -impl Housekeeper { +impl Housekeeper where T: 'static { pub(crate) fn new(inner: Weak) -> Self { use crate::common::thread_pool::PoolName; @@ -107,7 +107,7 @@ impl Housekeeper { fn start_periodical_sync_job( thread_pool: &Arc, - unsafe_weak_ptr: Arc>, + unsafe_weak_ptr: Arc>>, is_shutting_down: Arc, periodical_sync_running: Arc>, ) -> JobHandle { @@ -173,10 +173,10 @@ impl Housekeeper { // private functions/methods impl Housekeeper { - fn call_sync(unsafe_weak_ptr: &Arc>) -> Option { + fn call_sync(unsafe_weak_ptr: &Arc>>) -> Option { let lock = unsafe_weak_ptr.lock(); // Restore the Weak pointer to Inner. - let weak = unsafe { lock.as_weak_arc::() }; + let weak = unsafe { lock.as_weak_arc() }; if let Some(inner) = weak.upgrade() { // TODO: Protect this call with catch_unwind(). let sync_pace = inner.sync(MAX_SYNC_REPEATS); diff --git a/src/sync/invalidator.rs b/src/sync/invalidator.rs index 7d153296..d24f77c9 100644 --- a/src/sync/invalidator.rs +++ b/src/sync/invalidator.rs @@ -288,7 +288,7 @@ impl Invalidator { struct ScanContext { predicates: Mutex>>, - cache: Mutex, + cache: Mutex>>, result: Mutex>>, is_running: AtomicBool, is_shutting_down: AtomicBool, @@ -373,7 +373,7 @@ where let cache_lock = self.scan_context.cache.lock(); // Restore the Weak pointer to Inner. - let weak = unsafe { cache_lock.as_weak_arc::>() }; + let weak = unsafe { cache_lock.as_weak_arc() }; if let Some(inner_cache) = weak.upgrade() { // TODO: Protect this call with catch_unwind(). *self.scan_context.result.lock() = Some(self.do_execute(&inner_cache)); From 41e85ddfc50588c06cf31902279584bbd46f96c7 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Thu, 19 May 2022 10:17:57 -0400 Subject: [PATCH 2/2] Revert crossbeam-epoch, cargo fmt --- Cargo.toml | 2 +- src/sync/housekeeper.rs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0c8a2087..2c777810 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ unstable-debug-counters = ["future"] [dependencies] crossbeam-channel = "0.5.4" -crossbeam-epoch = "0.9" +crossbeam-epoch = "0.8.2" crossbeam-utils = "0.8" num_cpus = "1.13" once_cell = "1.7" diff --git a/src/sync/housekeeper.rs b/src/sync/housekeeper.rs index 62705014..9a64dd4b 100644 --- a/src/sync/housekeeper.rs +++ b/src/sync/housekeeper.rs @@ -78,7 +78,10 @@ impl Drop for Housekeeper { } // functions/methods used by Cache -impl Housekeeper where T: 'static { +impl Housekeeper +where + T: 'static, +{ pub(crate) fn new(inner: Weak) -> Self { use crate::common::thread_pool::PoolName;