Merge pull request #19 from moka-rs/add-send-sync-static-bounds

Add Send + Sync + 'static bounds to K, V and S of sync and future Caches
moka-rs · Jun 13, 2021 · e04d2ef · e04d2ef
2 parents b656c28 + 8c7cc9b
commit e04d2ef
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 75 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,16 @@
 # Moka &mdash; Change Log
 
+## Version 0.4.0
+
+### Fixed
+
+- **Breaking change**: Now `sync::{Cache, SegmentedCache}` and `future::Cache`
+  require `Send`, `Sync` and `'static` bounds for the generic parameters `K` (key),
+  `V` (value) and `S` (hasher state). This is necessary to prevent potential
+  undefined behaviors in applications using single-threaded async runtime such as
+  Actix-rt. ([#19][gh-pull-0019])
+
+
 ## Version 0.3.1
 
 ### Changed
@@ -49,6 +60,7 @@
 
 [caffeine-git]: https://github.com/ben-manes/caffeine
 
+[gh-pull-0019]: https://github.com/moka-rs/moka/pull/19/
 [gh-pull-0016]: https://github.com/moka-rs/moka/pull/16/
 [gh-pull-0011]: https://github.com/moka-rs/moka/pull/11/
 [gh-pull-0009]: https://github.com/moka-rs/moka/pull/9/

diff --git a/src/future/builder.rs b/src/future/builder.rs
@@ -47,8 +47,8 @@ pub struct CacheBuilder<C> {
 
 impl<K, V> CacheBuilder<Cache<K, V, RandomState>>
 where
-    K: Eq + Hash,
-    V: Clone,
+    K: Eq + Hash + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
 {
     /// Construct a new `CacheBuilder` that will be used to build a `Cache` holding
     /// up to `max_capacity` entries.
@@ -78,7 +78,7 @@ where
     /// Builds a `Cache<K, V, S>`, with the given `hasher`.
     pub fn build_with_hasher<S>(self, hasher: S) -> Cache<K, V, S>
     where
-        S: BuildHasher + Clone,
+        S: BuildHasher + Clone + Send + Sync + 'static,
     {
         Cache::with_everything(
             self.max_capacity,

diff --git a/src/future/cache.rs b/src/future/cache.rs
@@ -107,20 +107,15 @@ use std::{
 ///     }
 /// }
 /// ```
+///
 /// # Thread Safety
 ///
 /// All methods provided by the `Cache` are considered thread-safe, and can be safely
 /// accessed by multiple concurrent threads.
 ///
-/// `Cache<K, V, S>` will implement `Send` when all of the following conditions meet:
-///
-/// - `K` (key) and `V` (value) implement `Send` and `Sync`.
-/// - `S` (the hash-map state) implements `Send`.
-///
-/// and will implement `Sync` when all of the following conditions meet:
-///
-/// - `K` (key) and `V` (value) implement `Send` and `Sync`.
-/// - `S` (the hash-map state) implements `Sync`.
+/// - `Cache<K, V, S>` requires trait bounds `Send`, `Sync` and `'static` for `K`
+///   (key), `V` (value) and `S` (hasher state).
+/// - `Cache<K, V, S>` will implement `Send` and `Sync`.
 ///
 /// # Sharing a cache across asynchronous tasks
 ///
@@ -209,8 +204,8 @@ where
 
 impl<K, V> Cache<K, V, RandomState>
 where
-    K: Hash + Eq,
-    V: Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
 {
     /// Constructs a new `Cache<K, V>` that will store up to the `max_capacity` entries.
     ///
@@ -226,9 +221,9 @@ where
 
 impl<K, V, S> Cache<K, V, S>
 where
-    K: Hash + Eq,
-    V: Clone,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     pub(crate) fn with_everything(
         max_capacity: usize,
@@ -366,8 +361,9 @@ where
 
 impl<K, V, S> ConcurrentCacheExt<K, V> for Cache<K, V, S>
 where
-    K: Hash + Eq,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     fn sync(&self) {
         self.base.inner.sync(MAX_SYNC_REPEATS);
@@ -377,9 +373,9 @@ where
 // private methods
 impl<K, V, S> Cache<K, V, S>
 where
-    K: Hash + Eq,
-    V: Clone,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     async fn insert_with_hash(&self, key: K, hash: u64, value: V) {
         let op = self.base.do_insert_with_hash(key, hash, value);
@@ -444,8 +440,9 @@ where
 #[cfg(test)]
 impl<K, V, S> Cache<K, V, S>
 where
-    K: Hash + Eq,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     fn reconfigure_for_testing(&mut self) {
         self.base.reconfigure_for_testing();

diff --git a/src/sync/base_cache.rs b/src/sync/base_cache.rs
@@ -74,9 +74,9 @@ impl<K, V, S> Drop for BaseCache<K, V, S> {
 
 impl<K, V, S> BaseCache<K, V, S>
 where
-    K: Hash + Eq,
-    V: Clone,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     pub(crate) fn new(
         max_capacity: usize,
@@ -196,9 +196,9 @@ where
 //
 impl<K, V, S> BaseCache<K, V, S>
 where
-    K: Hash + Eq,
-    V: Clone,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     #[inline]
     fn record_read_op(&self, op: ReadOp<K, V>) -> Result<(), TrySendError<ReadOp<K, V>>> {
@@ -296,8 +296,9 @@ where
 #[cfg(test)]
 impl<K, V, S> BaseCache<K, V, S>
 where
-    K: Hash + Eq,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     pub(crate) fn is_empty(&self) -> bool {
         self.inner.len() == 0
@@ -463,8 +464,9 @@ where
 
 impl<K, V, S> InnerSync for Inner<K, V, S>
 where
-    K: Hash + Eq,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     fn sync(&self, max_repeats: usize) -> Option<SyncPace> {
         const EVICTION_BATCH_SIZE: usize = 500;
@@ -509,8 +511,9 @@ where
 //
 impl<K, V, S> Inner<K, V, S>
 where
-    K: Hash + Eq,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     fn apply_reads(&self, deqs: &mut Deques<K>, count: usize) {
         use ReadOp::*;

diff --git a/src/sync/builder.rs b/src/sync/builder.rs
@@ -49,8 +49,8 @@ pub struct CacheBuilder<C> {
 
 impl<K, V> CacheBuilder<Cache<K, V, RandomState>>
 where
-    K: Eq + Hash,
-    V: Clone,
+    K: Eq + Hash + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
 {
     /// Construct a new `CacheBuilder` that will be used to build a `Cache` or
     /// `SegmentedCache` holding up to `max_capacity` entries.
@@ -104,7 +104,7 @@ where
     /// calling this method.
     pub fn build_with_hasher<S>(self, hasher: S) -> Cache<K, V, S>
     where
-        S: BuildHasher + Clone,
+        S: BuildHasher + Clone + Send + Sync + 'static,
     {
         Cache::with_everything(
             self.max_capacity,
@@ -118,8 +118,8 @@ where
 
 impl<K, V> CacheBuilder<SegmentedCache<K, V, RandomState>>
 where
-    K: Eq + Hash,
-    V: Clone,
+    K: Eq + Hash + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
 {
     /// Builds a `SegmentedCache<K, V>`.
     ///
@@ -143,7 +143,7 @@ where
     /// calling this method.
     pub fn build_with_hasher<S>(self, hasher: S) -> SegmentedCache<K, V, S>
     where
-        S: BuildHasher + Clone,
+        S: BuildHasher + Clone + Send + Sync + 'static,
     {
         SegmentedCache::with_everything(
             self.max_capacity,

diff --git a/src/sync/cache.rs b/src/sync/cache.rs
@@ -90,15 +90,9 @@ use std::{
 /// All methods provided by the `Cache` are considered thread-safe, and can be safely
 /// accessed by multiple concurrent threads.
 ///
-/// `Cache<K, V, S>` will implement `Send` when all of the following conditions meet:
-///
-/// - `K` (key) and `V` (value) implement `Send` and `Sync`.
-/// - `S` (the hash-map state) implements `Send`.
-///
-/// and will implement `Sync` when all of the following conditions meet:
-///
-/// - `K` (key) and `V` (value) implement `Send` and `Sync`.
-/// - `S` (the hash-map state) implements `Sync`.
+/// - `Cache<K, V, S>` requires trait bounds `Send`, `Sync` and `'static` for `K`
+///   (key), `V` (value) and `S` (hasher state).
+/// - `Cache<K, V, S>` will implement `Send` and `Sync`.
 ///
 /// # Sharing a cache across threads
 ///
@@ -187,8 +181,9 @@ where
 
 impl<K, V> Cache<K, V, RandomState>
 where
-    K: Hash + Eq,
-    V: Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
 {
     /// Constructs a new `Cache<K, V>` that will store up to the `max_capacity` entries.
     ///
@@ -204,9 +199,9 @@ where
 
 impl<K, V, S> Cache<K, V, S>
 where
-    K: Hash + Eq,
-    V: Clone,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     pub(crate) fn with_everything(
         max_capacity: usize,
@@ -321,8 +316,9 @@ where
 
 impl<K, V, S> ConcurrentCacheExt<K, V> for Cache<K, V, S>
 where
-    K: Hash + Eq,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     fn sync(&self) {
         self.base.inner.sync(MAX_SYNC_REPEATS);
@@ -332,9 +328,9 @@ where
 // private methods
 impl<K, V, S> Cache<K, V, S>
 where
-    K: Hash + Eq,
-    V: Clone,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     #[inline]
     fn schedule_write_op(
@@ -367,8 +363,9 @@ where
 #[cfg(test)]
 impl<K, V, S> Cache<K, V, S>
 where
-    K: Hash + Eq,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     pub(crate) fn reconfigure_for_testing(&mut self) {
         self.base.reconfigure_for_testing();

diff --git a/src/sync/segment.rs b/src/sync/segment.rs
@@ -52,8 +52,8 @@ impl<K, V, S> Clone for SegmentedCache<K, V, S> {
 
 impl<K, V> SegmentedCache<K, V, RandomState>
 where
-    K: Hash + Eq,
-    V: Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
 {
     /// Constructs a new `SegmentedCache<K, V>` that has multiple internal
     /// segments and will store up to the `max_capacity` entries.
@@ -74,9 +74,9 @@ where
 
 impl<K, V, S> SegmentedCache<K, V, S>
 where
-    K: Hash + Eq,
-    V: Clone,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     /// # Panics
     ///
@@ -179,8 +179,9 @@ where
 
 impl<K, V, S> ConcurrentCacheExt<K, V> for SegmentedCache<K, V, S>
 where
-    K: Hash + Eq,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     fn sync(&self) {
         for segment in self.inner.segments.iter() {
@@ -193,8 +194,9 @@ where
 #[cfg(test)]
 impl<K, V, S> SegmentedCache<K, V, S>
 where
-    K: Hash + Eq,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     fn reconfigure_for_testing(&mut self) {
         let inner = Arc::get_mut(&mut self.inner)
@@ -215,9 +217,9 @@ struct Inner<K, V, S> {
 
 impl<K, V, S> Inner<K, V, S>
 where
-    K: Hash + Eq,
-    V: Clone,
-    S: BuildHasher + Clone,
+    K: Hash + Eq + Send + Sync + 'static,
+    V: Clone + Send + Sync + 'static,
+    S: BuildHasher + Clone + Send + Sync + 'static,
 {
     /// # Panics
     ///