Skip to content

Commit

Permalink
Store the debt list as pointers, not refs
Browse files Browse the repository at this point in the history
The idea is that the whole list (tail of the list) is synchronized by
the access of the head. Nevertheless, we fill in the next pointer before
that and having a reference is formally considered a read of the
destination. Even though we _use_ the reference only after the
synchronization point, we have it before and MIRI considers it a data
race.

Using a pointer is fine, as dereferencing is manual and considered read
only at that point.
  • Loading branch information
vorner committed Dec 25, 2022
1 parent fd04e20 commit 97a65cd
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
* Fix a data race reported by MIRI.
* Avoid violating stacked borrows (AFAIK these are still experimental and not
normative, but better safe than sorry). (#80).
* The `AccessConvert` wrapper is needed less often in practice (#77).
Expand Down
13 changes: 9 additions & 4 deletions src/debt/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ pub(crate) struct Node {
fast: FastSlots,
helping: HelpingSlots,
in_use: AtomicUsize,
next: Option<&'static Node>,
// Next node in the list.
//
// It is a pointer because we touch it before synchronization (we don't _dereference_ it before
// synchronization, only manipulate the pointer itself). That is illegal according to strict
// interpretation of the rules by MIRI on references.
next: *const Node,
active_writers: AtomicUsize,
}

Expand All @@ -68,7 +73,7 @@ impl Default for Node {
fast: FastSlots::default(),
helping: HelpingSlots::default(),
in_use: AtomicUsize::new(NODE_USED),
next: None,
next: ptr::null(),
active_writers: AtomicUsize::new(0),
}
}
Expand All @@ -94,7 +99,7 @@ impl Node {
if result.is_some() {
return result;
}
current = node.next;
current = unsafe { node.next.as_ref() };
}
None
}
Expand Down Expand Up @@ -165,7 +170,7 @@ impl Node {
// compare_exchange below.
let mut head = LIST_HEAD.load(Relaxed);
loop {
node.next = unsafe { head.as_ref() };
node.next = head;
if let Err(old) = LIST_HEAD.compare_exchange_weak(
head, node,
// We need to release *the whole chain* here. For that, we need to
Expand Down

0 comments on commit 97a65cd

Please sign in to comment.