Skip to content

Commit 2dc91f0

Browse files
Avoid invalid Send trait in evicting_map.
The evict_items was async which caused an invalid Send trait to be required on functions that use it. This was detected by wrapping the asyncio Mutex with a std::sync::Mutex such that the compiler could detect the invalid usage. There was a valid usage by manually calling drop(state) in get and size_for_key, however, a bug in the compiler (rust-lang/rust#57478) meant that refactoring this allows the compiler to better understand the lock.
1 parent 14700ed commit 2dc91f0

File tree

1 file changed

+29
-24
lines changed

1 file changed

+29
-24
lines changed

util/evicting_map.rs

+29-24
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ pub struct EvictingMap<T: LenEntry + Debug, I: InstantWrapper> {
115115

116116
impl<T, I> EvictingMap<T, I>
117117
where
118-
T: LenEntry + Debug + Clone + Send + Sync,
118+
T: LenEntry + Debug + Clone + Send + Sync + 'static,
119119
I: InstantWrapper,
120120
{
121121
pub fn new(config: &EvictionPolicy, anchor_time: I) -> Self {
@@ -168,7 +168,7 @@ where
168168
);
169169
}
170170
// Just in case we allow for some cleanup (eg: old items).
171-
self.evict_items(state.deref_mut()).await;
171+
self.evict_items(state.deref_mut());
172172
}
173173

174174
fn should_evict(&self, lru_len: usize, peek_entry: &EvictionItem<T>, sum_store_size: u64, evicting: bool) -> bool {
@@ -184,13 +184,10 @@ where
184184

185185
let is_over_count = self.max_count != 0 && (lru_len as u64) > self.max_count;
186186

187-
if is_over_size || old_item_exists || is_over_count {
188-
return true;
189-
}
190-
false
187+
is_over_size || old_item_exists || is_over_count
191188
}
192189

193-
async fn evict_items(&self, state: &mut State<T>) {
190+
fn evict_items(&self, state: &mut State<T>) {
194191
let mut peek_entry = if let Some((_, entry)) = state.lru.peek_lru() {
195192
entry
196193
} else {
@@ -201,8 +198,13 @@ where
201198
evicting = true;
202199
let (key, eviction_item) = state.lru.pop_lru().expect("Tried to peek() then pop() but failed");
203200
state.sum_store_size -= eviction_item.data.len() as u64;
204-
eviction_item.data.unref().await;
205-
log::info!("\x1b[0;31mevicting map\x1b[0m: Evicting {:?}", key);
201+
202+
// Perform the eviction in a separate greenlet because this function
203+
// cannot be async otherwise the lock Send trait does not hold.
204+
tokio::spawn(async move {
205+
eviction_item.data.unref().await;
206+
log::info!("\x1b[0;31mevicting map\x1b[0m: Evicting {:?}", key);
207+
});
206208

207209
peek_entry = if let Some((_, entry)) = state.lru.peek_lru() {
208210
entry
@@ -212,28 +214,31 @@ where
212214
}
213215
}
214216

215-
pub async fn size_for_key(&self, digest: &DigestInfo) -> Option<usize> {
217+
async fn get_item_for_key(&self, digest: &DigestInfo) -> Option<T> {
216218
let mut state = self.state.lock().await;
217219
if let Some(mut entry) = state.lru.get_mut(digest) {
218220
entry.seconds_since_anchor = self.anchor_time.elapsed().as_secs() as i32;
219-
let data = entry.data.clone();
220-
drop(state);
221-
data.touch().await;
222-
return Some(data.len());
221+
Some(entry.data.clone())
222+
} else {
223+
None
224+
}
225+
}
226+
227+
pub async fn size_for_key(&self, digest: &DigestInfo) -> Option<usize> {
228+
match self.get(digest).await {
229+
Some(data) => Some(data.len()),
230+
None => None,
223231
}
224-
None
225232
}
226233

227234
pub async fn get(&self, digest: &DigestInfo) -> Option<T> {
228-
let mut state = self.state.lock().await;
229-
if let Some(mut entry) = state.lru.get_mut(digest) {
230-
entry.seconds_since_anchor = self.anchor_time.elapsed().as_secs() as i32;
231-
let data = entry.data.clone();
232-
drop(state);
233-
data.touch().await;
234-
return Some(data);
235+
match self.get_item_for_key(digest).await {
236+
Some(data) => {
237+
data.touch().await;
238+
Some(data)
239+
}
240+
None => None,
235241
}
236-
None
237242
}
238243

239244
/// Returns the replaced item if any.
@@ -264,7 +269,7 @@ where
264269
None
265270
};
266271
state.sum_store_size += new_item_size;
267-
self.evict_items(state.deref_mut()).await;
272+
self.evict_items(state.deref_mut());
268273
maybe_old_item
269274
}
270275

0 commit comments

Comments
 (0)