From f498f91deeb782a7df79e5b54262344cbf6e907b Mon Sep 17 00:00:00 2001 From: Tatsuya Kawano Date: Sat, 20 Jan 2024 08:05:43 +0800 Subject: [PATCH 1/5] Ensure `crossbeam-epoch` to run GC when dropping a cache --- src/future/base_cache.rs | 14 ++++++++++++++ src/future/cache.rs | 12 ++++++++++++ src/sync/cache.rs | 13 +++++++++++++ src/sync_base/base_cache.rs | 14 ++++++++++++++ 4 files changed, 53 insertions(+) diff --git a/src/future/base_cache.rs b/src/future/base_cache.rs index 41e94aac..f8e6f4f0 100644 --- a/src/future/base_cache.rs +++ b/src/future/base_cache.rs @@ -86,6 +86,20 @@ impl Drop for BaseCache { fn drop(&mut self) { // The housekeeper needs to be dropped before the inner is dropped. std::mem::drop(self.housekeeper.take()); + + // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the + // global bag so that previously cached values will be dropped. + + // First call should push the `deferred_fn`s in the thread local storage + // (TLS) of the current thread to the global bag. + crossbeam_epoch::pin().flush(); + // Second call should collect the `deferred_fn`s in the global bag. + crossbeam_epoch::pin().flush(); + + // NOTE: inner, read_op_ch, and write_op_ch will be dropped after returning + // from this `drop` method. They use crossbeam-epoch internally, but we do + // not have to call `flush` for them because their `drop` methods do not + // create `deferred_fn`s, and drop their values in place. } } diff --git a/src/future/cache.rs b/src/future/cache.rs index 84d1c615..523f49cd 100644 --- a/src/future/cache.rs +++ b/src/future/cache.rs @@ -5454,6 +5454,18 @@ mod tests { assert_eq!(counters.value_dropped(), KEYS, "value_dropped"); } + // https://github.com/moka-rs/moka/issues/383 + #[tokio::test] + async fn ensure_gc_runs_when_dropping_cache() { + let cache = Cache::builder().build(); + let val = Arc::new(0); + cache + .get_with(1, std::future::ready(Arc::clone(&val))) + .await; + drop(cache); + assert_eq!(Arc::strong_count(&val), 1); + } + #[tokio::test] async fn test_debug_format() { let cache = Cache::new(10); diff --git a/src/sync/cache.rs b/src/sync/cache.rs index 4a154bdb..50b3f116 100644 --- a/src/sync/cache.rs +++ b/src/sync/cache.rs @@ -4831,6 +4831,19 @@ mod tests { assert_eq!(counters.value_dropped(), KEYS, "value_dropped"); } + // https://github.com/moka-rs/moka/issues/383 + #[test] + fn ensure_gc_runs_when_dropping_cache() { + let cache = Cache::builder().build(); + let val = Arc::new(0); + { + let val = Arc::clone(&val); + cache.get_with(1, move || val); + } + drop(cache); + assert_eq!(Arc::strong_count(&val), 1); + } + #[test] fn test_debug_format() { let cache = Cache::new(10); diff --git a/src/sync_base/base_cache.rs b/src/sync_base/base_cache.rs index 0d4571e1..11bfdc20 100644 --- a/src/sync_base/base_cache.rs +++ b/src/sync_base/base_cache.rs @@ -75,6 +75,20 @@ impl Drop for BaseCache { fn drop(&mut self) { // The housekeeper needs to be dropped before the inner is dropped. std::mem::drop(self.housekeeper.take()); + + // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the + // global bag so that previously cached values will be dropped. + + // First call should push the `deferred_fn`s in the thread local storage + // (TLS) of the current thread to the global bag. + crossbeam_epoch::pin().flush(); + // Second call should collect the `deferred_fn`s in the global bag. + crossbeam_epoch::pin().flush(); + + // NOTE: inner, read_op_ch, and write_op_ch will be dropped after returning + // from this `drop` method. They use crossbeam-epoch internally, but we do + // not have to call `flush` for them because their `drop` methods do not + // create `deferred_fn`s, and drop their values in place. } } From 6b79f5839a79a0d18a337adfcfe7ae4a2586fbf9 Mon Sep 17 00:00:00 2001 From: Tatsuya Kawano Date: Sat, 20 Jan 2024 09:11:15 +0800 Subject: [PATCH 2/5] Ensure crossbeam-epoch to run GC when dropping a cache --- src/future/base_cache.rs | 9 +++------ src/sync_base/base_cache.rs | 9 +++------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/future/base_cache.rs b/src/future/base_cache.rs index f8e6f4f0..8409588b 100644 --- a/src/future/base_cache.rs +++ b/src/future/base_cache.rs @@ -89,12 +89,9 @@ impl Drop for BaseCache { // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the // global bag so that previously cached values will be dropped. - - // First call should push the `deferred_fn`s in the thread local storage - // (TLS) of the current thread to the global bag. - crossbeam_epoch::pin().flush(); - // Second call should collect the `deferred_fn`s in the global bag. - crossbeam_epoch::pin().flush(); + for _ in 0..10 { + crossbeam_epoch::pin().flush(); + } // NOTE: inner, read_op_ch, and write_op_ch will be dropped after returning // from this `drop` method. They use crossbeam-epoch internally, but we do diff --git a/src/sync_base/base_cache.rs b/src/sync_base/base_cache.rs index 11bfdc20..04a55326 100644 --- a/src/sync_base/base_cache.rs +++ b/src/sync_base/base_cache.rs @@ -78,12 +78,9 @@ impl Drop for BaseCache { // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the // global bag so that previously cached values will be dropped. - - // First call should push the `deferred_fn`s in the thread local storage - // (TLS) of the current thread to the global bag. - crossbeam_epoch::pin().flush(); - // Second call should collect the `deferred_fn`s in the global bag. - crossbeam_epoch::pin().flush(); + for _ in 0..10 { + crossbeam_epoch::pin().flush(); + } // NOTE: inner, read_op_ch, and write_op_ch will be dropped after returning // from this `drop` method. They use crossbeam-epoch internally, but we do From 8488c4908b0ce09d577d85d122f972139c9e50c0 Mon Sep 17 00:00:00 2001 From: Tatsuya Kawano Date: Sat, 20 Jan 2024 10:42:05 +0800 Subject: [PATCH 3/5] Ensure crossbeam-epoch to run GC when dropping a cache --- src/future/base_cache.rs | 2 +- src/sync_base/base_cache.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/future/base_cache.rs b/src/future/base_cache.rs index 8409588b..232c85b3 100644 --- a/src/future/base_cache.rs +++ b/src/future/base_cache.rs @@ -89,7 +89,7 @@ impl Drop for BaseCache { // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the // global bag so that previously cached values will be dropped. - for _ in 0..10 { + for _ in 0..128 { crossbeam_epoch::pin().flush(); } diff --git a/src/sync_base/base_cache.rs b/src/sync_base/base_cache.rs index 04a55326..d66cc7e8 100644 --- a/src/sync_base/base_cache.rs +++ b/src/sync_base/base_cache.rs @@ -78,7 +78,7 @@ impl Drop for BaseCache { // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the // global bag so that previously cached values will be dropped. - for _ in 0..10 { + for _ in 0..128 { crossbeam_epoch::pin().flush(); } From 761899cf5a3d6cf8044ced9d92dc3e50b3cb64e8 Mon Sep 17 00:00:00 2001 From: Tatsuya Kawano Date: Sat, 20 Jan 2024 11:46:43 +0800 Subject: [PATCH 4/5] Ensure crossbeam-epoch to run GC when dropping a cache --- src/future/base_cache.rs | 21 ++++++++++++--------- src/sync_base/base_cache.rs | 20 +++++++++++--------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/future/base_cache.rs b/src/future/base_cache.rs index 232c85b3..cf3edd2a 100644 --- a/src/future/base_cache.rs +++ b/src/future/base_cache.rs @@ -87,16 +87,19 @@ impl Drop for BaseCache { // The housekeeper needs to be dropped before the inner is dropped. std::mem::drop(self.housekeeper.take()); - // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the - // global bag so that previously cached values will be dropped. - for _ in 0..128 { - crossbeam_epoch::pin().flush(); - } + if Arc::strong_count(&self.inner) <= 1 { + dbg!(Arc::strong_count(&self.inner)); + // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the + // global bag so that previously cached values will be dropped. + for _ in 0..128 { + crossbeam_epoch::pin().flush(); + } - // NOTE: inner, read_op_ch, and write_op_ch will be dropped after returning - // from this `drop` method. They use crossbeam-epoch internally, but we do - // not have to call `flush` for them because their `drop` methods do not - // create `deferred_fn`s, and drop their values in place. + // NOTE: The `inner` will be dropped after returning from this `drop` + // method. It uses crossbeam-epoch internally, but we do not have to call + // `flush` for it because its `drop` methods do not create + // `deferred_fn`s, and drop its values in place. + } } } diff --git a/src/sync_base/base_cache.rs b/src/sync_base/base_cache.rs index d66cc7e8..b18aff5d 100644 --- a/src/sync_base/base_cache.rs +++ b/src/sync_base/base_cache.rs @@ -76,16 +76,18 @@ impl Drop for BaseCache { // The housekeeper needs to be dropped before the inner is dropped. std::mem::drop(self.housekeeper.take()); - // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the - // global bag so that previously cached values will be dropped. - for _ in 0..128 { - crossbeam_epoch::pin().flush(); - } + if Arc::strong_count(&self.inner) <= 1 { + // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the + // global bag so that previously cached values will be dropped. + for _ in 0..128 { + crossbeam_epoch::pin().flush(); + } - // NOTE: inner, read_op_ch, and write_op_ch will be dropped after returning - // from this `drop` method. They use crossbeam-epoch internally, but we do - // not have to call `flush` for them because their `drop` methods do not - // create `deferred_fn`s, and drop their values in place. + // NOTE: The `inner` will be dropped after returning from this `drop` + // method. It uses crossbeam-epoch internally, but we do not have to call + // `flush` for it because its `drop` methods do not create + // `deferred_fn`s, and drop its values in place. + } } } From a79eae2551800779931319c66bf0eb00bafdfefc Mon Sep 17 00:00:00 2001 From: Tatsuya Kawano Date: Sat, 20 Jan 2024 12:13:35 +0800 Subject: [PATCH 5/5] Ensure crossbeam-epoch to run GC when dropping a cache --- src/future/base_cache.rs | 29 +++++++++++++++-------------- src/sync_base/base_cache.rs | 28 +++++++++++++++------------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/future/base_cache.rs b/src/future/base_cache.rs index cf3edd2a..fc733003 100644 --- a/src/future/base_cache.rs +++ b/src/future/base_cache.rs @@ -86,20 +86,6 @@ impl Drop for BaseCache { fn drop(&mut self) { // The housekeeper needs to be dropped before the inner is dropped. std::mem::drop(self.housekeeper.take()); - - if Arc::strong_count(&self.inner) <= 1 { - dbg!(Arc::strong_count(&self.inner)); - // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the - // global bag so that previously cached values will be dropped. - for _ in 0..128 { - crossbeam_epoch::pin().flush(); - } - - // NOTE: The `inner` will be dropped after returning from this `drop` - // method. It uses crossbeam-epoch internally, but we do not have to call - // `flush` for it because its `drop` methods do not create - // `deferred_fn`s, and drop its values in place. - } } } @@ -1064,6 +1050,21 @@ pub(crate) struct Inner { clocks: Clocks, } +impl Drop for Inner { + fn drop(&mut self) { + // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the + // global bag so that previously cached values will be dropped. + for _ in 0..128 { + crossbeam_epoch::pin().flush(); + } + + // NOTE: The `CacheStore` (`cht`) will be dropped after returning from this + // `drop` method. It uses crossbeam-epoch internally, but we do not have to + // call `flush` for it because its `drop` methods do not create + // `deferred_fn`s, and drop its values in place. + } +} + // // functions/methods used by BaseCache // diff --git a/src/sync_base/base_cache.rs b/src/sync_base/base_cache.rs index b18aff5d..304083dc 100644 --- a/src/sync_base/base_cache.rs +++ b/src/sync_base/base_cache.rs @@ -75,19 +75,6 @@ impl Drop for BaseCache { fn drop(&mut self) { // The housekeeper needs to be dropped before the inner is dropped. std::mem::drop(self.housekeeper.take()); - - if Arc::strong_count(&self.inner) <= 1 { - // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the - // global bag so that previously cached values will be dropped. - for _ in 0..128 { - crossbeam_epoch::pin().flush(); - } - - // NOTE: The `inner` will be dropped after returning from this `drop` - // method. It uses crossbeam-epoch internally, but we do not have to call - // `flush` for it because its `drop` methods do not create - // `deferred_fn`s, and drop its values in place. - } } } @@ -923,6 +910,21 @@ pub(crate) struct Inner { clocks: Clocks, } +impl Drop for Inner { + fn drop(&mut self) { + // Ensure crossbeam-epoch to collect garbages (`deferred_fn`s) in the + // global bag so that previously cached values will be dropped. + for _ in 0..128 { + crossbeam_epoch::pin().flush(); + } + + // NOTE: The `CacheStore` (`cht`) will be dropped after returning from this + // `drop` method. It uses crossbeam-epoch internally, but we do not have to + // call `flush` for it because its `drop` methods do not create + // `deferred_fn`s, and drop its values in place. + } +} + // // functions/methods used by BaseCache //