search

rust-lang / miri

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

Remove MutexID list #4002

Closed tiif closed 2 weeks ago

tiif commented 1 month ago

Remove MutexID list in SynchronizationObjects to avoid memory leaks.

cc #3967

tiif commented 4 weeks ago

I feel I have been constantly running into already mutably borrowed: BorrowError when it comes to unblock_thread. It always takes me a while to realise that it has something to do with unblock_thread again.

Usually, the list of blocked thread ids is stored in a struct. Alternating between retrieving thread id (which involves a mutable borrow to the struct) and unblock thread will usually lead to BorrowMut error because the struct often need to be borrowed again during the unblock callback.

RalfJung commented 4 weeks ago

Generally you have to take great care of your RefCell handles. Never keep them any longer than you have to. For every function you call while holding the handle, ensure it doesn't reentrantly access the same data again.

If we didn't use RefCell, you'd get a borrow checking error due to changing a vector while iterating it. That would be nicer but unfortunately the reference patterns are too dynamic to make that work.

tiif commented 4 weeks ago

For every function you call while holding the handle, ensure it doesn't reentrantly access the same data again.

This is a pretty nice way to see it, thanks.

RalfJung commented 2 weeks ago

@rustbot author

tiif commented 2 weeks ago

@rustbot ready

RalfJung commented 2 weeks ago

Just some final nits. :) You can squash after taking care of those (with --keep-base as usual).

@rustbot author

tiif commented 2 weeks ago

@rustbot ready

RalfJung commented 2 weeks ago

Oh, wtf... as long as there is a "requested changes" review, it won't merge, even if the merge button is pressed by the person that requested the changes...