Skip to content

Commit 0963353

Browse files
borsbors
committed
Auto merge of #3631 - RalfJung:blocking-refactor, r=RalfJung
completely refactor how we manage blocking and unblocking threads This hides a lot of invariants from the implementation of the synchronization primitives, and makes sure we never have to release or acquire a vector clock on another thread but the active one.
2 parents 8e861c6 + 2e89443 commit 0963353

19 files changed

+951
-1043
lines changed

‎src/tools/miri/src/alloc_addresses/mod.rs

+5-9
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,10 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
169169
size,
170170
align,
171171
memory_kind,
172-
ecx.get_active_thread(),
172+
ecx.active_thread(),
173173
) {
174-
if let Some(clock) = clock
175-
&& let Some(data_race) = &ecx.machine.data_race
176-
{
177-
data_race.acquire_clock(&clock, ecx.get_active_thread());
174+
if let Some(clock) = clock {
175+
ecx.acquire_clock(&clock);
178176
}
179177
reuse_addr
180178
} else {
@@ -369,12 +367,10 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
369367
// `alloc_id_from_addr` any more.
370368
global_state.exposed.remove(&dead_id);
371369
// Also remember this address for future reuse.
372-
let thread = self.threads.get_active_thread_id();
370+
let thread = self.threads.active_thread();
373371
global_state.reuse.add_addr(rng, addr, size, align, kind, thread, || {
374372
if let Some(data_race) = &self.data_race {
375-
data_race
376-
.release_clock(thread, self.threads.active_thread_ref().current_span())
377-
.clone()
373+
data_race.release_clock(&self.threads).clone()
378374
} else {
379375
VClock::default()
380376
}

‎src/tools/miri/src/concurrency/data_race.rs

+69-83
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ use std::{
4747
};
4848

4949
use rustc_ast::Mutability;
50-
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
50+
use rustc_data_structures::fx::FxHashSet;
5151
use rustc_index::{Idx, IndexVec};
5252
use rustc_middle::{mir, ty::Ty};
5353
use rustc_span::Span;
@@ -822,6 +822,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
822822
assert!(!old, "cannot nest allow_data_races");
823823
}
824824
}
825+
826+
/// Returns the `release` clock of the current thread.
827+
/// Other threads can acquire this clock in the future to establish synchronization
828+
/// with this program point.
829+
fn release_clock<'a>(&'a self) -> Option<Ref<'a, VClock>>
830+
where
831+
'mir: 'a,
832+
{
833+
let this = self.eval_context_ref();
834+
Some(this.machine.data_race.as_ref()?.release_clock(&this.machine.threads))
835+
}
836+
837+
/// Acquire the given clock into the current thread, establishing synchronization with
838+
/// the moment when that clock snapshot was taken via `release_clock`.
839+
fn acquire_clock(&self, clock: &VClock) {
840+
let this = self.eval_context_ref();
841+
if let Some(data_race) = &this.machine.data_race {
842+
data_race.acquire_clock(clock, &this.machine.threads);
843+
}
844+
}
825845
}
826846

827847
/// Vector clock metadata for a logical memory allocation.
@@ -1412,13 +1432,6 @@ pub struct GlobalState {
14121432
/// active vector-clocks catch up with the threads timestamp.
14131433
reuse_candidates: RefCell<FxHashSet<VectorIdx>>,
14141434

1415-
/// This contains threads that have terminated, but not yet joined
1416-
/// and so cannot become re-use candidates until a join operation
1417-
/// occurs.
1418-
/// The associated vector index will be moved into re-use candidates
1419-
/// after the join operation occurs.
1420-
terminated_threads: RefCell<FxHashMap<ThreadId, VectorIdx>>,
1421-
14221435
/// The timestamp of last SC fence performed by each thread
14231436
last_sc_fence: RefCell<VClock>,
14241437

@@ -1446,7 +1459,6 @@ impl GlobalState {
14461459
vector_info: RefCell::new(IndexVec::new()),
14471460
thread_info: RefCell::new(IndexVec::new()),
14481461
reuse_candidates: RefCell::new(FxHashSet::default()),
1449-
terminated_threads: RefCell::new(FxHashMap::default()),
14501462
last_sc_fence: RefCell::new(VClock::default()),
14511463
last_sc_write: RefCell::new(VClock::default()),
14521464
track_outdated_loads: config.track_outdated_loads,
@@ -1480,8 +1492,6 @@ impl GlobalState {
14801492
fn find_vector_index_reuse_candidate(&self) -> Option<VectorIdx> {
14811493
let mut reuse = self.reuse_candidates.borrow_mut();
14821494
let vector_clocks = self.vector_clocks.borrow();
1483-
let vector_info = self.vector_info.borrow();
1484-
let terminated_threads = self.terminated_threads.borrow();
14851495
for &candidate in reuse.iter() {
14861496
let target_timestamp = vector_clocks[candidate].clock[candidate];
14871497
if vector_clocks.iter_enumerated().all(|(clock_idx, clock)| {
@@ -1491,9 +1501,7 @@ impl GlobalState {
14911501

14921502
// The vector represents a thread that has terminated and hence cannot
14931503
// report a data-race with the candidate index.
1494-
let thread_id = vector_info[clock_idx];
1495-
let vector_terminated =
1496-
reuse.contains(&clock_idx) || terminated_threads.contains_key(&thread_id);
1504+
let vector_terminated = reuse.contains(&clock_idx);
14971505

14981506
// The vector index cannot report a race with the candidate index
14991507
// and hence allows the candidate index to be re-used.
@@ -1583,55 +1591,38 @@ impl GlobalState {
15831591
/// thread (the joinee, the thread that someone waited on) and the current thread (the joiner,
15841592
/// the thread who was waiting).
15851593
#[inline]
1586-
pub fn thread_joined(
1587-
&mut self,
1588-
thread_mgr: &ThreadManager<'_, '_>,
1589-
joiner: ThreadId,
1590-
joinee: ThreadId,
1591-
) {
1592-
let clocks_vec = self.vector_clocks.get_mut();
1593-
let thread_info = self.thread_info.get_mut();
1594-
1595-
// Load the vector clock of the current thread.
1596-
let current_index = thread_info[joiner]
1597-
.vector_index
1598-
.expect("Performed thread join on thread with no assigned vector");
1599-
let current = &mut clocks_vec[current_index];
1594+
pub fn thread_joined(&mut self, threads: &ThreadManager<'_, '_>, joinee: ThreadId) {
1595+
let thread_info = self.thread_info.borrow();
1596+
let thread_info = &thread_info[joinee];
16001597

16011598
// Load the associated vector clock for the terminated thread.
1602-
let join_clock = thread_info[joinee]
1599+
let join_clock = thread_info
16031600
.termination_vector_clock
16041601
.as_ref()
1605-
.expect("Joined with thread but thread has not terminated");
1606-
1607-
// The join thread happens-before the current thread
1608-
// so update the current vector clock.
1609-
// Is not a release operation so the clock is not incremented.
1610-
current.clock.join(join_clock);
1602+
.expect("joined with thread but thread has not terminated");
1603+
// Acquire that into the current thread.
1604+
self.acquire_clock(join_clock, threads);
16111605

16121606
// Check the number of live threads, if the value is 1
16131607
// then test for potentially disabling multi-threaded execution.
1614-
if thread_mgr.get_live_thread_count() == 1 {
1615-
// May potentially be able to disable multi-threaded execution.
1616-
let current_clock = &clocks_vec[current_index];
1617-
if clocks_vec
1618-
.iter_enumerated()
1619-
.all(|(idx, clocks)| clocks.clock[idx] <= current_clock.clock[idx])
1620-
{
1621-
// All thread terminations happen-before the current clock
1622-
// therefore no data-races can be reported until a new thread
1623-
// is created, so disable multi-threaded execution.
1624-
self.multi_threaded.set(false);
1608+
// This has to happen after `acquire_clock`, otherwise there'll always
1609+
// be some thread that has not synchronized yet.
1610+
if let Some(current_index) = thread_info.vector_index {
1611+
if threads.get_live_thread_count() == 1 {
1612+
let vector_clocks = self.vector_clocks.get_mut();
1613+
// May potentially be able to disable multi-threaded execution.
1614+
let current_clock = &vector_clocks[current_index];
1615+
if vector_clocks
1616+
.iter_enumerated()
1617+
.all(|(idx, clocks)| clocks.clock[idx] <= current_clock.clock[idx])
1618+
{
1619+
// All thread terminations happen-before the current clock
1620+
// therefore no data-races can be reported until a new thread
1621+
// is created, so disable multi-threaded execution.
1622+
self.multi_threaded.set(false);
1623+
}
16251624
}
16261625
}
1627-
1628-
// If the thread is marked as terminated but not joined
1629-
// then move the thread to the re-use set.
1630-
let termination = self.terminated_threads.get_mut();
1631-
if let Some(index) = termination.remove(&joinee) {
1632-
let reuse = self.reuse_candidates.get_mut();
1633-
reuse.insert(index);
1634-
}
16351626
}
16361627

16371628
/// On thread termination, the vector-clock may re-used
@@ -1642,29 +1633,18 @@ impl GlobalState {
16421633
/// This should be called strictly before any calls to
16431634
/// `thread_joined`.
16441635
#[inline]
1645-
pub fn thread_terminated(&mut self, thread_mgr: &ThreadManager<'_, '_>, current_span: Span) {
1636+
pub fn thread_terminated(&mut self, thread_mgr: &ThreadManager<'_, '_>) {
1637+
let current_thread = thread_mgr.active_thread();
16461638
let current_index = self.active_thread_index(thread_mgr);
16471639

1648-
// Increment the clock to a unique termination timestamp.
1649-
let vector_clocks = self.vector_clocks.get_mut();
1650-
let current_clocks = &mut vector_clocks[current_index];
1651-
current_clocks.increment_clock(current_index, current_span);
1652-
1653-
// Load the current thread id for the executing vector.
1654-
let vector_info = self.vector_info.get_mut();
1655-
let current_thread = vector_info[current_index];
1656-
1657-
// Load the current thread metadata, and move to a terminated
1658-
// vector state. Setting up the vector clock all join operations
1659-
// will use.
1660-
let thread_info = self.thread_info.get_mut();
1661-
let current = &mut thread_info[current_thread];
1662-
current.termination_vector_clock = Some(current_clocks.clock.clone());
1640+
// Store the terminaion clock.
1641+
let terminaion_clock = self.release_clock(thread_mgr).clone();
1642+
self.thread_info.get_mut()[current_thread].termination_vector_clock =
1643+
Some(terminaion_clock);
16631644

1664-
// Add this thread as a candidate for re-use after a thread join
1665-
// occurs.
1666-
let termination = self.terminated_threads.get_mut();
1667-
termination.insert(current_thread, current_index);
1645+
// Add this thread's clock index as a candidate for re-use.
1646+
let reuse = self.reuse_candidates.get_mut();
1647+
reuse.insert(current_index);
16681648
}
16691649

16701650
/// Attempt to perform a synchronized operation, this
@@ -1702,23 +1682,29 @@ impl GlobalState {
17021682
format!("thread `{thread_name}`")
17031683
}
17041684

1705-
/// Acquire the given clock into the given thread, establishing synchronization with
1685+
/// Acquire the given clock into the current thread, establishing synchronization with
17061686
/// the moment when that clock snapshot was taken via `release_clock`.
17071687
/// As this is an acquire operation, the thread timestamp is not
17081688
/// incremented.
1709-
pub fn acquire_clock(&self, lock: &VClock, thread: ThreadId) {
1689+
pub fn acquire_clock<'mir, 'tcx>(&self, clock: &VClock, threads: &ThreadManager<'mir, 'tcx>) {
1690+
let thread = threads.active_thread();
17101691
let (_, mut clocks) = self.thread_state_mut(thread);
1711-
clocks.clock.join(lock);
1692+
clocks.clock.join(clock);
17121693
}
17131694

1714-
/// Returns the `release` clock of the given thread.
1695+
/// Returns the `release` clock of the current thread.
17151696
/// Other threads can acquire this clock in the future to establish synchronization
17161697
/// with this program point.
1717-
pub fn release_clock(&self, thread: ThreadId, current_span: Span) -> Ref<'_, VClock> {
1698+
pub fn release_clock<'mir, 'tcx>(
1699+
&self,
1700+
threads: &ThreadManager<'mir, 'tcx>,
1701+
) -> Ref<'_, VClock> {
1702+
let thread = threads.active_thread();
1703+
let span = threads.active_thread_ref().current_span();
17181704
// We increment the clock each time this happens, to ensure no two releases
17191705
// can be confused with each other.
17201706
let (index, mut clocks) = self.thread_state_mut(thread);
1721-
clocks.increment_clock(index, current_span);
1707+
clocks.increment_clock(index, span);
17221708
drop(clocks);
17231709
// To return a read-only view, we need to release the RefCell
17241710
// and borrow it again.
@@ -1757,7 +1743,7 @@ impl GlobalState {
17571743
&self,
17581744
thread_mgr: &ThreadManager<'_, '_>,
17591745
) -> (VectorIdx, Ref<'_, ThreadClockSet>) {
1760-
self.thread_state(thread_mgr.get_active_thread_id())
1746+
self.thread_state(thread_mgr.active_thread())
17611747
}
17621748

17631749
/// Load the current vector clock in use and the current set of thread clocks
@@ -1767,14 +1753,14 @@ impl GlobalState {
17671753
&self,
17681754
thread_mgr: &ThreadManager<'_, '_>,
17691755
) -> (VectorIdx, RefMut<'_, ThreadClockSet>) {
1770-
self.thread_state_mut(thread_mgr.get_active_thread_id())
1756+
self.thread_state_mut(thread_mgr.active_thread())
17711757
}
17721758

17731759
/// Return the current thread, should be the same
17741760
/// as the data-race active thread.
17751761
#[inline]
17761762
fn active_thread_index(&self, thread_mgr: &ThreadManager<'_, '_>) -> VectorIdx {
1777-
let active_thread_id = thread_mgr.get_active_thread_id();
1763+
let active_thread_id = thread_mgr.active_thread();
17781764
self.thread_index(active_thread_id)
17791765
}
17801766

0 commit comments

Comments
 (0)