Fix race condition in interners (#4300)

This commit is contained in:
Laurenz 2024-05-30 14:46:47 +02:00 committed by GitHub
parent fa7fbb8274
commit 8a9c45e7d4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 15 additions and 8 deletions

View File

@ -36,17 +36,20 @@ impl FileId {
#[track_caller]
pub fn new(package: Option<PackageSpec>, path: VirtualPath) -> Self {
// Try to find an existing entry that we can reuse.
//
// We could check with just a read lock, but if the pair is not yet
// present, we would then need to recheck after acquiring a write lock,
// which is probably not worth it.
let pair = (package, path);
if let Some(&id) = INTERNER.read().unwrap().to_id.get(&pair) {
let mut interner = INTERNER.write().unwrap();
if let Some(&id) = interner.to_id.get(&pair) {
return id;
}
let mut interner = INTERNER.write().unwrap();
let num = interner.from_id.len().try_into().expect("out of file ids");
// Create a new entry forever by leaking the pair. We can't leak more
// than 2^16 pair (and typically will leak a lot less), so its not a
// big deal.
let num = interner.from_id.len().try_into().expect("out of file ids");
let id = FileId(num);
let leaked = Box::leak(Box::new(pair));
interner.to_id.insert(leaked, id);

View File

@ -27,15 +27,19 @@ pub struct PicoStr(u32);
impl PicoStr {
/// Creates a new interned string.
pub fn new(string: &str) -> Self {
if let Some(&id) = INTERNER.read().unwrap().to_id.get(string) {
// Try to find an existing entry that we can reuse.
//
// We could check with just a read lock, but if the string is not yet
// present, we would then need to recheck after acquiring a write lock,
// which is probably not worth it.
let mut interner = INTERNER.write().unwrap();
if let Some(&id) = interner.to_id.get(string) {
return id;
}
let mut interner = INTERNER.write().unwrap();
let num = interner.from_id.len().try_into().expect("out of string ids");
// Create a new entry forever by leaking the string. PicoStr is only
// used for strings that aren't created en masse, so it is okay.
let num = interner.from_id.len().try_into().expect("out of string ids");
let id = Self(num);
let string = Box::leak(string.to_string().into_boxed_str());
interner.to_id.insert(string, id);