firefly
firefly copied to clipboard
`Process::acquire_heap` should be made re-entrant
The documentation for ProcessControlBlock::acquire_heap is reentrant. This code seems to use the rust std Mutex, which is not.
https://github.com/lumen/lumen/blob/aeae1721d0fc02982898c8bcd1e7ed0ae5db151c/liblumen_alloc/src/erts/process.rs#L202-L204
This causes issues when (for instance) using ProcessControlBlock::tuple_from_iter along with an iterator that allocates.
Yeah, I saw the same behavior and mostly use the heap functions directly in process for that reason now.
The reason is that the lock used to be a ReentrantMutex, not a Mutex; at some point that changed but that comment wasn't updated. If it needs to be re-entrant, we should make the necessary changes to support using the former, but if not, then we can just remove that note from the docs.
I actually hit this during my work on #350, in that some tests were acquring a lock directly, then calling code which internally tried to acquire a lock. The result was that property tests would just hang for no apparent reason, in random places.
I think we need to either find a way to make this lock the Reentrant version again, or disallow acquiring the lock directly except in specific situations where we can guarantee re-entrancy isn't needed - otherwise it is going to be far too easy for code to slip in which introduces a deadlock.
No longer relevant