Skip to content

Commit 6ec74ec

Browse files
petrosaggtaiki-e
authored andcommitted
crossbeam-channel: prevent double free on Drop (#1187)
This PR is fixing a regression introduced by #1084 that can lead to a double free when dropping the channel. The method `Channel::discard_all_messages` has the property that if it observes `head.block` pointing to a non-null pointer it will attempt to free it. The same property holds for the `Channel::drop` method and so it is critical that whenever `head.block` is freed it must also be set to a null pointer so that it is freed exactly once. Before #1084 the behavior of `discard_all_messages` ensured `head.block` was `null` after its execution due to the atomic store right before exiting [1]. After #1084 `discard_all_messages` atomically swaps the current value of `head.block` with a null pointer at the moment the value is read instead of waiting for the end of the function. The problem lies in the fact that `dicard_all_messages` contained two paths that could lead to `head.block` being read but only one of them would swap the value. This meant that `dicard_all_messages` could end up observing a non-null block pointer (and therefore attempting to free it) without setting `head.block` to null. This would then lead to `Channel::drop` making a second attempt at dropping the same pointer. The bug is similar to the one previously fixed by #972 and the double free can be reproduced by reverting the reproduction commit from that PR [2]. As with #972 it is quite difficult to trigger this bug without introducing artificial sleeps in critical points so this PR does not include a test. [1] https://github.com/crossbeam-rs/crossbeam/blob/crossbeam-channel-0.5.11/crossbeam-channel/src/flavors/list.rs#L625 [2] 2d22628 Signed-off-by: Petros Angelatos <[email protected]>
1 parent ccd83ac commit 6ec74ec

File tree

1 file changed

+1
-1
lines changed
  • crossbeam-channel/src/flavors

1 file changed

+1
-1
lines changed

crossbeam-channel/src/flavors/list.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,7 @@ impl<T> Channel<T> {
611611
// In that case, just wait until it gets initialized.
612612
while block.is_null() {
613613
backoff.snooze();
614-
block = self.head.block.load(Ordering::Acquire);
614+
block = self.head.block.swap(ptr::null_mut(), Ordering::AcqRel);
615615
}
616616
}
617617

0 commit comments

Comments
 (0)