Skip to content

Commit 7049aa6

Browse files
shamb0shamb0
committed
concurrency: Generalize UnblockCallback to MachineCallback
 * Introduce MachineCallbackState enum to represent operation outcomes * Consolidate unblock/timeout methods into single callback interface * Update thread blocking system to use new callback mechanism * Refactor mutex and condvar implementations for new callback pattern Signed-off-by: shamb0 <[email protected]>
1 parent 97517d0 commit 7049aa6

File tree

10 files changed

+191
-134
lines changed

10 files changed

+191
-134
lines changed

‎src/concurrency/sync.rs‎

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
422422
mutex_ref: MutexRef,
423423
retval_dest: Option<(Scalar, MPlaceTy<'tcx>)>,
424424
}
425-
@unblock = |this| {
425+
@unblock = |this, unblock: UnblockKind| {
426+
assert_eq!(unblock, UnblockKind::Ready);
427+
426428
assert!(!this.mutex_is_locked(&mutex_ref));
427429
this.mutex_lock(&mutex_ref);
428430

@@ -538,7 +540,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
538540
retval: Scalar,
539541
dest: MPlaceTy<'tcx>,
540542
}
541-
@unblock = |this| {
543+
@unblock = |this, unblock: UnblockKind| {
544+
assert_eq!(unblock, UnblockKind::Ready);
542545
this.rwlock_reader_lock(id);
543546
this.write_scalar(retval, &dest)?;
544547
interp_ok(())
@@ -623,7 +626,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
623626
retval: Scalar,
624627
dest: MPlaceTy<'tcx>,
625628
}
626-
@unblock = |this| {
629+
@unblock = |this, unblock: UnblockKind| {
630+
assert_eq!(unblock, UnblockKind::Ready);
627631
this.rwlock_writer_lock(id);
628632
this.write_scalar(retval, &dest)?;
629633
interp_ok(())
@@ -677,25 +681,29 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
677681
retval_timeout: Scalar,
678682
dest: MPlaceTy<'tcx>,
679683
}
680-
@unblock = |this| {
681-
// The condvar was signaled. Make sure we get the clock for that.
682-
if let Some(data_race) = &this.machine.data_race {
683-
data_race.acquire_clock(
684-
&this.machine.sync.condvars[condvar].clock,
685-
&this.machine.threads,
686-
);
684+
@unblock = |this, unblock: UnblockKind| {
685+
match unblock {
686+
UnblockKind::Ready => {
687+
// The condvar was signaled. Make sure we get the clock for that.
688+
if let Some(data_race) = &this.machine.data_race {
689+
data_race.acquire_clock(
690+
&this.machine.sync.condvars[condvar].clock,
691+
&this.machine.threads,
692+
);
693+
}
694+
// Try to acquire the mutex.
695+
// The timeout only applies to the first wait (until the signal), not for mutex acquisition.
696+
this.condvar_reacquire_mutex(&mutex_ref, retval_succ, dest)
697+
}
698+
UnblockKind::TimedOut => {
699+
// We have to remove the waiter from the queue again.
700+
let thread = this.active_thread();
701+
let waiters = &mut this.machine.sync.condvars[condvar].waiters;
702+
waiters.retain(|waiter| *waiter != thread);
703+
// Now get back the lock.
704+
this.condvar_reacquire_mutex(&mutex_ref, retval_timeout, dest)
705+
}
687706
}
688-
// Try to acquire the mutex.
689-
// The timeout only applies to the first wait (until the signal), not for mutex acquisition.
690-
this.condvar_reacquire_mutex(&mutex_ref, retval_succ, dest)
691-
}
692-
@timeout = |this| {
693-
// We have to remove the waiter from the queue again.
694-
let thread = this.active_thread();
695-
let waiters = &mut this.machine.sync.condvars[condvar].waiters;
696-
waiters.retain(|waiter| *waiter != thread);
697-
// Now get back the lock.
698-
this.condvar_reacquire_mutex(&mutex_ref, retval_timeout, dest)
699707
}
700708
),
701709
);
@@ -752,25 +760,29 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
752760
dest: MPlaceTy<'tcx>,
753761
errno_timeout: IoError,
754762
}
755-
@unblock = |this| {
756-
let futex = futex_ref.0.borrow();
757-
// Acquire the clock of the futex.
758-
if let Some(data_race) = &this.machine.data_race {
759-
data_race.acquire_clock(&futex.clock, &this.machine.threads);
763+
@unblock = |this, unblock: UnblockKind| {
764+
match unblock {
765+
UnblockKind::Ready => {
766+
let futex = futex_ref.0.borrow();
767+
// Acquire the clock of the futex.
768+
if let Some(data_race) = &this.machine.data_race {
769+
data_race.acquire_clock(&futex.clock, &this.machine.threads);
770+
}
771+
// Write the return value.
772+
this.write_scalar(retval_succ, &dest)?;
773+
interp_ok(())
774+
},
775+
UnblockKind::TimedOut => {
776+
// Remove the waiter from the futex.
777+
let thread = this.active_thread();
778+
let mut futex = futex_ref.0.borrow_mut();
779+
futex.waiters.retain(|waiter| waiter.thread != thread);
780+
// Set errno and write return value.
781+
this.set_last_error(errno_timeout)?;
782+
this.write_scalar(retval_timeout, &dest)?;
783+
interp_ok(())
784+
},
760785
}
761-
// Write the return value.
762-
this.write_scalar(retval_succ, &dest)?;
763-
interp_ok(())
764-
}
765-
@timeout = |this| {
766-
// Remove the waiter from the futex.
767-
let thread = this.active_thread();
768-
let mut futex = futex_ref.0.borrow_mut();
769-
futex.waiters.retain(|waiter| waiter.thread != thread);
770-
// Set errno and write return value.
771-
this.set_last_error(errno_timeout)?;
772-
this.write_scalar(retval_timeout, &dest)?;
773-
interp_ok(())
774786
}
775787
),
776788
);

‎src/concurrency/thread.rs‎

Lines changed: 15 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -38,71 +38,20 @@ pub enum TlsAllocAction {
3838
Leak,
3939
}
4040

41-
/// Trait for callbacks that are executed when a thread gets unblocked.
42-
pub trait UnblockCallback<'tcx>: VisitProvenance {
43-
/// Will be invoked when the thread was unblocked the "regular" way,
44-
/// i.e. whatever event it was blocking on has happened.
45-
fn unblock(self: Box<Self>, ecx: &mut InterpCx<'tcx, MiriMachine<'tcx>>) -> InterpResult<'tcx>;
46-
47-
/// Will be invoked when the timeout ellapsed without the event the
48-
/// thread was blocking on having occurred.
49-
fn timeout(self: Box<Self>, _ecx: &mut InterpCx<'tcx, MiriMachine<'tcx>>)
50-
-> InterpResult<'tcx>;
41+
/// The argument type for the "unblock" callback, indicating why the thread got unblocked.
42+
#[derive(Debug, PartialEq)]
43+
pub enum UnblockKind {
44+
/// Operation completed successfully, thread continues normal execution.
45+
Ready,
46+
/// The operation did not complete within its specified duration.
47+
TimedOut,
5148
}
52-
pub type DynUnblockCallback<'tcx> = Box<dyn UnblockCallback<'tcx> + 'tcx>;
53-
54-
#[macro_export]
55-
macro_rules! callback {
56-
(
57-
@capture<$tcx:lifetime $(,)? $($lft:lifetime),*> { $($name:ident: $type:ty),* $(,)? }
58-
@unblock = |$this:ident| $unblock:block
59-
) => {
60-
callback!(
61-
@capture<$tcx, $($lft),*> { $($name: $type),* }
62-
@unblock = |$this| $unblock
63-
@timeout = |_this| {
64-
unreachable!(
65-
"timeout on a thread that was blocked without a timeout (or someone forgot to overwrite this method)"
66-
)
67-
}
68-
)
69-
};
70-
(
71-
@capture<$tcx:lifetime $(,)? $($lft:lifetime),*> { $($name:ident: $type:ty),* $(,)? }
72-
@unblock = |$this:ident| $unblock:block
73-
@timeout = |$this_timeout:ident| $timeout:block
74-
) => {{
75-
struct Callback<$tcx, $($lft),*> {
76-
$($name: $type,)*
77-
_phantom: std::marker::PhantomData<&$tcx ()>,
78-
}
79-
80-
impl<$tcx, $($lft),*> VisitProvenance for Callback<$tcx, $($lft),*> {
81-
#[allow(unused_variables)]
82-
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
83-
$(
84-
self.$name.visit_provenance(visit);
85-
)*
86-
}
87-
}
88-
89-
impl<$tcx, $($lft),*> UnblockCallback<$tcx> for Callback<$tcx, $($lft),*> {
90-
fn unblock(self: Box<Self>, $this: &mut MiriInterpCx<$tcx>) -> InterpResult<$tcx> {
91-
#[allow(unused_variables)]
92-
let Callback { $($name,)* _phantom } = *self;
93-
$unblock
94-
}
9549

96-
fn timeout(self: Box<Self>, $this_timeout: &mut MiriInterpCx<$tcx>) -> InterpResult<$tcx> {
97-
#[allow(unused_variables)]
98-
let Callback { $($name,)* _phantom } = *self;
99-
$timeout
100-
}
101-
}
50+
/// Type alias for boxed machine callbacks with generic argument type.
51+
pub type DyMachineCallback<'tcx, T> = Box<dyn MachineCallback<'tcx, T> + 'tcx>;
10252

103-
Box::new(Callback { $($name,)* _phantom: std::marker::PhantomData })
104-
}}
105-
}
53+
/// Type alias for unblock callbacks using UnblockKind argument.
54+
pub type DynUnblockCallback<'tcx> = DyMachineCallback<'tcx, UnblockKind>;
10655

10756
/// A thread identifier.
10857
#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)]
@@ -656,7 +605,8 @@ impl<'tcx> ThreadManager<'tcx> {
656605
@capture<'tcx> {
657606
joined_thread_id: ThreadId,
658607
}
659-
@unblock = |this| {
608+
@unblock = |this, unblock: UnblockKind| {
609+
assert_eq!(unblock, UnblockKind::Ready);
660610
if let Some(data_race) = &mut this.machine.data_race {
661611
data_race.thread_joined(&this.machine.threads, joined_thread_id);
662612
}
@@ -842,7 +792,7 @@ trait EvalContextPrivExt<'tcx>: MiriInterpCxExt<'tcx> {
842792
// 2. Make the scheduler the only place that can change the active
843793
// thread.
844794
let old_thread = this.machine.threads.set_active_thread_id(thread);
845-
callback.timeout(this)?;
795+
callback.call(this, UnblockKind::TimedOut)?;
846796
this.machine.threads.set_active_thread_id(old_thread);
847797
}
848798
// found_callback can remain None if the computer's clock
@@ -1084,7 +1034,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
10841034
};
10851035
// The callback must be executed in the previously blocked thread.
10861036
let old_thread = this.machine.threads.set_active_thread_id(thread);
1087-
callback.unblock(this)?;
1037+
callback.call(this, UnblockKind::Ready)?;
10881038
this.machine.threads.set_active_thread_id(old_thread);
10891039
interp_ok(())
10901040
}

‎src/lib.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ pub use crate::concurrency::sync::{
126126
CondvarId, EvalContextExt as _, MutexRef, RwLockId, SynchronizationObjects,
127127
};
128128
pub use crate::concurrency::thread::{
129-
BlockReason, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager, TimeoutAnchor,
130-
TimeoutClock, UnblockCallback,
129+
BlockReason, DynUnblockCallback, EvalContextExt as _, StackEmptyCallback, ThreadId,
130+
ThreadManager, TimeoutAnchor, TimeoutClock, UnblockKind,
131131
};
132132
pub use crate::diagnostics::{
133133
EvalContextExt as _, NonHaltingDiagnostic, TerminationInfo, report_error,
@@ -139,8 +139,8 @@ pub use crate::eval::{
139139
pub use crate::helpers::{AccessKind, EvalContextExt as _};
140140
pub use crate::intrinsics::EvalContextExt as _;
141141
pub use crate::machine::{
142-
AllocExtra, FrameExtra, MemoryKind, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind,
143-
PrimitiveLayouts, Provenance, ProvenanceExtra,
142+
AllocExtra, FrameExtra, MachineCallback, MemoryKind, MiriInterpCx, MiriInterpCxExt,
143+
MiriMachine, MiriMemoryKind, PrimitiveLayouts, Provenance, ProvenanceExtra,
144144
};
145145
pub use crate::mono_hash_map::MonoHashMap;
146146
pub use crate::operator::EvalContextExt as _;

‎src/machine.rs‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1723,3 +1723,77 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
17231723
Cow::Borrowed(ecx.machine.union_data_ranges.entry(ty).or_insert_with(compute_range))
17241724
}
17251725
}
1726+
1727+
/// Trait for callbacks handling asynchronous machine operations.
1728+
///
1729+
/// Callbacks receive a completion state and can perform follow-up actions while
1730+
/// maintaining interpreter invariants. They are executed with mutable access to
1731+
/// the interpreter context.
1732+
///
1733+
/// # Type Parameters
1734+
/// - `'tcx`: Typing context lifetime for the interpreter.
1735+
/// - `T`: Type of argument passed to the callback on completion.
1736+
pub trait MachineCallback<'tcx, T>: VisitProvenance {
1737+
/// Executes the callback when an operation completes.
1738+
///
1739+
/// # Arguments
1740+
/// - `self`: Owned callback, boxed for dynamic dispatch
1741+
/// - `ecx`: Mutable interpreter context
1742+
/// - `arg`: Operation-specific completion argument
1743+
///
1744+
/// # Returns
1745+
/// Success or error of the callback execution.
1746+
fn call(
1747+
self: Box<Self>,
1748+
ecx: &mut InterpCx<'tcx, MiriMachine<'tcx>>,
1749+
arg: T,
1750+
) -> InterpResult<'tcx>;
1751+
}
1752+
1753+
/// Creates machine callbacks with captured variables and generic argument types.
1754+
///
1755+
/// This macro generates the boilerplate needed to create type-safe callbacks:
1756+
/// - Creates a struct to hold captured variables
1757+
/// - Implements required traits (VisitProvenance, MachineCallback)
1758+
/// - Handles proper lifetime and type parameters
1759+
///
1760+
/// The callback body receives the interpreter context and completion argument.
1761+
#[macro_export]
1762+
macro_rules! callback {
1763+
(@capture<$tcx:lifetime $(,)? $($lft:lifetime),*>
1764+
{ $($name:ident: $type:ty),* $(,)? }
1765+
@unblock = |$this:ident, $arg:ident: $arg_ty:ty| $body:expr $(,)?) => {{
1766+
// Create callback struct with the captured variables and generic type T
1767+
struct Callback<$tcx, $($lft),*> {
1768+
$($name: $type,)*
1769+
_phantom: std::marker::PhantomData<&$tcx ()>,
1770+
}
1771+
1772+
// Implement VisitProvenance trait for the callback
1773+
impl<$tcx, $($lft),*> VisitProvenance for Callback<$tcx, $($lft),*> {
1774+
fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
1775+
$(
1776+
self.$name.visit_provenance(_visit);
1777+
)*
1778+
}
1779+
}
1780+
1781+
// Implement MachineCallback trait with the specified argument type
1782+
impl<$tcx, $($lft),*> MachineCallback<$tcx, $arg_ty> for Callback<$tcx, $($lft),*> {
1783+
fn call(
1784+
self: Box<Self>,
1785+
$this: &mut MiriInterpCx<$tcx>,
1786+
$arg: $arg_ty
1787+
) -> InterpResult<$tcx> {
1788+
#[allow(unused_variables)]
1789+
let Callback { $($name,)* _phantom } = *self;
1790+
$body
1791+
}
1792+
}
1793+
1794+
Box::new(Callback {
1795+
$($name,)*
1796+
_phantom: std::marker::PhantomData
1797+
})
1798+
}};
1799+
}

‎src/shims/time.rs‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
331331
Some((TimeoutClock::Monotonic, TimeoutAnchor::Relative, duration)),
332332
callback!(
333333
@capture<'tcx> {}
334-
@unblock = |_this| { panic!("sleeping thread unblocked before time is up") }
335-
@timeout = |_this| { interp_ok(()) }
334+
@unblock = |_this, unblock: UnblockKind| {
335+
match unblock {
336+
UnblockKind::Ready => {
337+
panic!("sleeping thread unblocked before time is up")
338+
},
339+
UnblockKind::TimedOut => { interp_ok(()) },
340+
}
341+
}
336342
),
337343
);
338344
interp_ok(Scalar::from_i32(0))
@@ -353,8 +359,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
353359
Some((TimeoutClock::Monotonic, TimeoutAnchor::Relative, duration)),
354360
callback!(
355361
@capture<'tcx> {}
356-
@unblock = |_this| { panic!("sleeping thread unblocked before time is up") }
357-
@timeout = |_this| { interp_ok(()) }
362+
@unblock = |_this, unblock: UnblockKind| {
363+
match unblock {
364+
UnblockKind::Ready => {
365+
panic!("sleeping thread unblocked before time is up")
366+
},
367+
UnblockKind::TimedOut => { interp_ok(()) },
368+
}
369+
}
358370
),
359371
);
360372
interp_ok(())

0 commit comments

Comments
 (0)