Skip to content

Commit c088bf8

Browse files
committed
Refine timer callback feature gating and design comments
Remove redundant unsafe impl Send for ShareableMessageHandler.
1 parent 635a9a8 commit c088bf8

File tree

5 files changed

+17
-8
lines changed

5 files changed

+17
-8
lines changed

crates/common/src/ffi/timer.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ impl From<TimeEventHandlerV2> for TimeEventHandler {
4949
Self {
5050
event: value.event,
5151
callback_ptr: match value.callback {
52+
#[cfg(feature = "python")]
5253
TimeEventCallback::Python(callback) => callback.as_ptr().cast::<c_char>(),
5354
TimeEventCallback::Rust(_) => {
5455
panic!("Legacy time event handler is not supported for Rust callback")

crates/common/src/msgbus/handler.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ fn generate_handler_id<T: 'static + ?Sized, F: 'static + Fn(&T)>(callback: &F) -
135135
Ustr::from(&format!("<{callback_ptr:?}>-{uuid}"))
136136
}
137137

138+
// ShareableMessageHandler contains Rc<dyn MessageHandler> which is not Send/Sync.
139+
// This is intentional - message handlers are designed for single-threaded use within
140+
// each async runtime. The MessageBus uses thread-local storage to ensure each thread
141+
// gets its own handlers, eliminating the need for unsafe Send/Sync implementations.
138142
#[repr(transparent)]
139143
#[derive(Clone)]
140144
pub struct ShareableMessageHandler(pub Rc<dyn MessageHandler>);
@@ -159,7 +163,3 @@ impl From<Rc<dyn MessageHandler>> for ShareableMessageHandler {
159163
Self(value)
160164
}
161165
}
162-
163-
// SAFETY: Message handlers cannot be sent across thread boundaries
164-
#[allow(unsafe_code)]
165-
unsafe impl Send for ShareableMessageHandler {}

crates/common/src/python/clock.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ use crate::{
3333
#[allow(non_camel_case_types)]
3434
#[pyo3::pyclass(
3535
module = "nautilus_trader.core.nautilus_pyo3.common",
36-
name = "TestClock"
36+
name = "TestClock",
37+
unsendable
3738
)]
3839
#[derive(Debug)]
3940
pub struct TestClock_Py(Box<TestClock>);
@@ -126,7 +127,8 @@ impl TestClock_Py {
126127
#[allow(non_camel_case_types)]
127128
#[pyo3::pyclass(
128129
module = "nautilus_trader.core.nautilus_pyo3.common",
129-
name = "LiveClock"
130+
name = "LiveClock",
131+
unsendable
130132
)]
131133
#[derive(Debug)]
132134
pub struct LiveClock_Py(Box<LiveClock>);

crates/common/src/python/timer.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ impl From<TimeEventHandlerV2> for TimeEventHandler_Py {
5858
Self {
5959
event: value.event,
6060
callback: match value.callback {
61+
#[cfg(feature = "python")]
6162
TimeEventCallback::Python(callback) => callback,
6263
TimeEventCallback::Rust(_) => {
6364
panic!("Python time event handler is not supported for Rust callback")
@@ -193,7 +194,7 @@ mod tests {
193194
};
194195
use pyo3::prelude::*;
195196
use tokio::time::Duration;
196-
use ustr::Ustr; // Import required
197+
use ustr::Ustr; // Import required (due feature gating)
197198

198199
use crate::{
199200
testing::wait_until,

crates/common/src/timer.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,12 @@ impl From<PyObject> for TimeEventCallback {
180180
}
181181
}
182182

183-
// SAFETY: Message handlers cannot be sent across thread boundaries
183+
// TimeEventCallback supports both single-threaded and async use cases:
184+
// - Python variant uses Arc<PyObject> for cross-thread compatibility with Python's GIL
185+
// - Rust variant uses Rc<dyn Fn(TimeEvent)> for efficient single-threaded callbacks
186+
// SAFETY: The async timer tasks only use Python callbacks, and Rust callbacks are never
187+
// sent across thread boundaries in practice. This unsafe implementation allows the enum
188+
// to be moved into async tasks while maintaining the efficient Rc for single-threaded use.
184189
#[allow(unsafe_code)]
185190
unsafe impl Send for TimeEventCallback {}
186191
#[allow(unsafe_code)]

0 commit comments

Comments
 (0)