rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.4k stars 341 forks source link

Get rid of IDs for synchronization primitives, make pthread shims not leaky #3967

Open RalfJung opened 18 hours ago

RalfJung commented 18 hours ago

We currently store the state of our synchronization primitives with a layer of indirection: there are lists in SynchronizationObjects and then we use IDs to index into these lists. These lists keep growing while the program runs and never get cleaned up, which isn't very clean and also makes @saethlin unhappy. ;)

As of https://github.com/rust-lang/miri/pull/3966, we no longer store these IDs in machine memory, so there isn't actually a good reason to still use IDs. Instead, the PthreadMutex used by pthread_mutex_t can just directly store a sync::Mutex. That way, when the allocation that contains the mutex is freed, we also free all the extra data associated with the mutex, thus avoiding memory leaks.

The main trouble here is functions like mutex_unlock(&mut self, id: MutexId): these would want to take a reference to the Mutex instead, but since they also take a mutable reference to the entire machine state, that can't work! We could carefully arrange things to avoid overlapping references here... but probably the easier approach is to use reference counting. So we'd make PthreadMutex store a MutexRef (which is a newtype for Rc<RefCell<Mutex>>) and the function signatures would look like

fn mutex_unlock(&mut self, id: &MutexRef) -> ...

3966 does this for Futex so that can serve as a model.

Currently we are also using these IDs in BlockReason, but that's just a sanity check, so it's okay to remove them there.

Also as part of this, we could finally fix the leaks in pthread_mutex_destroy and friends.

tiif commented 17 hours ago

I still have a few things to clear in my todo list, but I'd like to work on a different part of the codebase, so,

@rustbot claim

RalfJung commented 17 hours ago

Note that this is blocked on https://github.com/rust-lang/miri/pull/3966