Skip to content

Commit c215b64

Browse files
committed
Fixed undefined behavior when using TLS
The mutable reference created by UnsafeCell stayed alive across the recursive call to inner_fn_call, which resulted in a mutable alias. To ensure that this kind of issue results in a panic rather than undefined behavior, I've switched to using a RefCell instead of an UnsafeCell. The RefMut obtained from the RefCell is explicitly dropped before we make the recursively call, ensuring that at most one is alive at any time.
1 parent 56cb601 commit c215b64

File tree

1 file changed

+13
-11
lines changed

1 file changed

+13
-11
lines changed

src/lib.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -287,26 +287,28 @@ fn build_tls_cache_body(full_cache_type: &syn::Type, cache_new: &syn::Expr,
287287
{
288288
parse_quote! {
289289
{
290-
use std::cell::UnsafeCell;
290+
use std::cell::RefCell;
291291
use std::thread_local;
292292
thread_local!(
293-
// We use `UnsafeCell` here to allow recursion. Since it is in the TLS, it should
294-
// not introduce any actual unsafety.
295-
static cache: UnsafeCell<#full_cache_type> =
296-
UnsafeCell::new(#cache_new);
293+
static cache: RefCell<#full_cache_type> =
294+
RefCell::new(#cache_new);
297295
);
298296
cache.with(|c| {
299-
let mut cache_ref = unsafe { &mut *c.get() };
297+
let mut cache_ref = c.borrow_mut();
300298
let cloned_args = #cloned_args;
301299

302300
let stored_result = cache_ref.get_mut(&cloned_args);
303301
if let Some(stored_result) = stored_result {
304-
stored_result.clone()
305-
} else {
306-
let ret = #inner_fn_call;
307-
cache_ref.insert(cloned_args, ret.clone());
308-
ret
302+
return stored_result.clone()
309303
}
304+
305+
// Don't hold a mutable borrow across
306+
// the recursive function call
307+
drop(cache_ref);
308+
309+
let ret = #inner_fn_call;
310+
c.borrow_mut().insert(cloned_args, ret.clone());
311+
ret
310312
})
311313
}
312314
}

0 commit comments

Comments
 (0)