sync: fix underflow in mpsc channel len()#8062
sync: fix underflow in mpsc channel len()#8062Darksonn wants to merge 2 commits intotokio-rs:masterfrom
len()#8062Conversation
Darksonn
changed the title
len()
Darksonn
force-pushed
the
mpsc-len-underflow
branch
from
e0ee027 to
a0a47ea
Compare
| // but it will be closed | ||
| let tail_position = tx.tail_position.load(Acquire); | ||
| tail_position - self.index - (tx.is_closed() as usize) | ||
| let mut len = tail_position.wrapping_sub(self.index); |
There was a problem hiding this comment.
I realized that we should also be using wrapping arithmetic for the position indices. On 32-bit machines these indices can wrap around after a few billion messages.
| } | ||
|
|
||
| fn has_value(&self, slot_index: usize) -> bool { | ||
| let block = unsafe { self.find_block(slot_index).as_ref() }; |
There was a problem hiding this comment.
Should we add a safety comment here?
| #[test] | ||
| fn len_accurate_during_close() { | ||
| loom::model(|| { | ||
| let (tx, rx) = mpsc::channel::<()>(2); |
There was a problem hiding this comment.
This test would be more interesting if it sends BLOCK_CAP messages before closing. This will test the edge case where block_tail advances to a new block during close().
| // Note that it is also possible for the ready bit to be unset on a normal message, but | ||
| // this happens only if that message is currently being sent *right now* in parallel on | ||
| // another thread. That is okay because it is optional to count messages that are currently | ||
| // being sent. |
There was a problem hiding this comment.
Is it possible another thread to advance the block_tail between line 271 (tail_position.load()) and line 285 (tx.has_value()) ?
If this happens tx.has_value() may enter an infinite loop calling block.grow() in find_block().
There was a problem hiding this comment.
The loop in find_block can only run finitely many times because each iteration advances to the next block, and it can only move to the next block offset/32 times.
3 participants
Fixes: #8061