Fix memory leak of MpMcQueue
#483
Conversation
sosthene-nitrokey
force-pushed
the
mpmc-drop
branch
from
d1eac57
to
7cfdb84
Compare
| impl<T, const N: usize> Drop for MpMcQueue<T, N> { | ||
| fn drop(&mut self) { | ||
| // drop all contents currently in the queue | ||
| while self.dequeue().is_some() {} |
There was a problem hiding this comment.
This is copying the elements to the stack before dropping them, and running the complex dequeue() logic.
it'd be more efficient to read enqueue_pos/dequeue_pos, iterate, then .assume_init_drop() all the items in place. No copies, no atomic ops. This is sound because on drop we have &mut self so we know no other thread can be concurrently accessing the queue at this point.
There was a problem hiding this comment.
I would like to implement that but I'm not 100% certain I understand the layout of the Queue which is not documented, and which items are actually valid. I'm certain that the dequeue approach is correct so this is the approach I took.
There was a problem hiding this comment.
ah yes, it's true it's not that trivial. We can optimize later if someone complains.
There was a problem hiding this comment.
Let's open an Issue for that
Fix #482