diff --git a/weather-station-cpp/src/lib.rs b/weather-station-cpp/src/lib.rs index 2b36abb..601b29d 100644 --- a/weather-station-cpp/src/lib.rs +++ b/weather-station-cpp/src/lib.rs @@ -1,35 +1,30 @@ //! The C ABI door onto [`StationHandle`], built as a spike. //! //! Not the shipped library: no soname, no CMake package config, no generated -//! header. It is the pendant of `weather-station-py` — the experiment that finds -//! out what an FFI layer needs from the station crates before they reach a -//! registry, run for the language where the boundary is a C ABI rather than an -//! interpreter. Findings are in `README.md`. +//! header. The pendant of `weather-station-py`, for the language where the +//! boundary is a C ABI rather than an interpreter. Findings are in `README.md`. //! //! # Three rules this layer exists to keep //! //! **Nothing unwinds across the boundary.** A Rust panic that reaches a C++ //! frame is undefined behaviour — there is no pyo3 here to turn it into an //! exception object. Every `extern "C"` function below wraps its body in -//! [`catch_unwind`](std::panic::catch_unwind) and reports [`WS_ERR_PANIC`], -//! and every callback this layer *invokes* is documented `noexcept` on the C++ -//! side for the same reason in the other direction. +//! [`catch_unwind`](std::panic::catch_unwind) and reports [`WS_ERR_PANIC`], and +//! every callback this layer *invokes* is documented `noexcept` on the C++ side +//! for the same reason in the other direction. //! //! **Every argument is hostile.** C has no `Option`, no lifetime and no UTF-8 //! guarantee, so a null pointer, a dangling one and a `const char*` that is not -//! UTF-8 all arrive as ordinary calls. Only the first two of those are -//! detectable; both are, here. +//! UTF-8 all arrive as ordinary calls. Only the first and third are detectable; +//! both are, here. //! //! **The callback thread is aimdb's runtime thread.** Once [`ws_init_logging`] //! is installed, aimdb's runtime thread calls out through a function pointer -//! into the consuming application. That is the same lock ordering the Python -//! door found — the GIL rewritten as "whatever lock the callback takes" — and -//! it is *weaker* here only because C++ has no single process-wide lock to get -//! wrong. See `README.md`. - -// The exported types carry their C names. A Rust-side `WsStation` aliased to -// `ws_station` would put two spellings of one type in front of whoever reads -// this file next to the header. +//! into the consuming application — the Python door's lock ordering, rewritten +//! as "whatever lock the callback takes". See `README.md`. + +// The exported types carry their C names, so the header and this file spell +// each type the same way. #![allow(non_camel_case_types)] use std::cell::RefCell; @@ -48,9 +43,8 @@ use tracing_subscriber::EnvFilter; use weather_station::{StationError, StationErrorKind, StationHandle, PROFILE_VERSION}; /// `ws_station` is handed to C as a pointer and used from several threads at -/// once, so the type behind it has to be `Send + Sync` for the same reason the -/// pyclass did — with nothing to check it, since C has no bound to violate. -/// Pinned here because an FFI layer is the only consumer that would notice. +/// once, so the type behind it has to be `Send + Sync` — with nothing on the C +/// side to check it. Pinned here instead. const _: fn() = || { fn assert_send_sync() {} assert_send_sync::(); @@ -88,10 +82,9 @@ pub const WS_ABI_VERSION: u32 = 1; thread_local! { /// Where the message goes when the return value is only a code. /// - /// Thread-local because the alternative is a global one two publishing - /// threads overwrite for each other. Owned by this layer and freed on the - /// next failing call from the same thread, which is the contract - /// `strerror_r`-style APIs have taught C callers to expect. + /// Thread-local because the alternative is a global two publishing threads + /// overwrite for each other. Owned by this layer and freed on the next + /// failing call from the same thread. static LAST_ERROR: RefCell> = const { RefCell::new(None) }; } @@ -117,8 +110,7 @@ fn clear_last_error() { /// /// Dispatches on `kind()` rather than the variants: `StationError` is /// `#[non_exhaustive]`, so matching it here would need a wildcard and a variant -/// added later would land in it silently — with no compiler to notice, since -/// the C caller's `switch` has a `default` too. +/// added later would land in it silently. fn report(err: StationError) -> c_int { set_last_error(err.to_string()); match err.kind() { @@ -132,9 +124,8 @@ fn report(err: StationError) -> c_int { /// behaviour. /// /// The one place this layer can be sure a panic stops. `AssertUnwindSafe` is -/// honest rather than convenient: a panic mid-call may leave the station in a -/// state no C caller can reason about, which is why the code says so instead of -/// pretending the call simply failed. +/// deliberate: a panic mid-call may leave the station in a state no C caller +/// can reason about, hence a distinct code rather than an ordinary failure. fn guard(body: impl FnOnce() -> c_int) -> c_int { match catch_unwind(AssertUnwindSafe(body)) { Ok(code) => code, @@ -173,10 +164,9 @@ unsafe fn cstr(ptr: *const c_char) -> Option<&'static str> { /// A station's seat in the mesh, behind a pointer. /// -/// `name` is owned here rather than borrowed from the slot: `MeshSlot::name` -/// returns a `&str`, which is a pointer *and a length*, and C wants a NUL. The -/// copy is made once at open so [`ws_station_name`] can hand back a pointer -/// that stays valid until [`ws_station_free`] without allocating per call. +/// `name` is owned here rather than borrowed from the slot, which returns a +/// `&str` where C wants a NUL. Copied once at open so [`ws_station_name`] can +/// hand back a pointer valid until [`ws_station_free`] without allocating. pub struct ws_station { inner: StationHandle, name: CString, @@ -221,11 +211,8 @@ pub extern "C" fn ws_last_error() -> *const c_char { /// `*out` owns a station the caller must eventually pass to /// [`ws_station_free`]. /// -/// `path` is bytes, and this layer requires them to be UTF-8. That is a real -/// constraint rather than a formality: Rust's `Path` is UTF-8 on every platform -/// this mesh targets, while a Windows console hands out UTF-16, so a shipped -/// library needs a `_w` entry point or a documented encoding rule. Recorded in -/// `README.md`. +/// `path` must be UTF-8. A shipped library would need a `_w` entry point or a +/// documented encoding rule for Windows; recorded in `README.md`. /// /// # Safety /// `path` must be a NUL-terminated string and `out` a writable pointer. @@ -266,9 +253,9 @@ pub unsafe extern "C" fn ws_station_open_profile( /// Refuse a call that needs a live runtime, with a message that says so. /// -/// Best-effort, exactly as the Python door's `ensure_open` is: `is_closed` -/// reads an atomic, so a close racing this check merely means the call fails -/// one layer down with aimdb's own "runtime thread has shut down". +/// Best-effort, exactly as the Python door's `ensure_open` is: a close racing +/// this check merely means the call fails one layer down with aimdb's own +/// "runtime thread has shut down". fn ensure_open(station: &ws_station) -> Option { if station.inner.is_closed() { set_last_error("this station is closed"); @@ -321,7 +308,7 @@ publish_fn!( /// The slot number this station publishes into, or `0` for a null handle. /// /// Still answers after a close: the slot comes from the profile, not the -/// runtime, and a closed station is a thing a log line still wants to identify. +/// runtime, and a closed station is still worth naming in a log line. /// /// # Safety /// `handle` must be null or a live station pointer. @@ -351,8 +338,8 @@ pub unsafe extern "C" fn ws_station_name(handle: *const ws_station) -> *const c_ /// Whether the station has been closed. `true` for a null handle, because a /// station that does not exist is not open. /// -/// Reads an atomic, never a lock: this is what a caller asks while holding its -/// own lock, and the answer must not queue behind a shutdown. +/// Never takes a lock: this is what a caller asks while holding its own, and +/// the answer must not queue behind a shutdown. /// /// # Safety /// `handle` must be null or a live station pointer. @@ -369,8 +356,6 @@ pub unsafe extern "C" fn ws_station_is_closed(handle: *const ws_station) -> bool /// /// Takes `const ws_station*` on purpose: `StationHandle::shutdown` takes /// `&self`, so this needs no exclusive access to a handle a publish is using. -/// That property is the one the Python door had to have fixed in -/// `weather-station` — this layer inherits it rather than re-solving it. /// /// A reading published in the last milliseconds before this does not /// necessarily arrive; see `README.md`. @@ -400,10 +385,8 @@ pub unsafe extern "C" fn ws_station_close(handle: *const ws_station) -> c_int { /// sit unguarded in a destructor. /// /// **Not thread-safe against anything else on the same pointer.** Every other -/// entry point takes a shared reference and may be called from any thread at -/// any time; this one consumes the allocation, and C has no borrow checker to -/// say so. It is the one place the C ABI is weaker than the Python door, where -/// the interpreter's own reference count decided when the object died. +/// entry point takes a shared reference and may be called from any thread; this +/// one consumes the allocation, and C has no borrow checker to say so. /// /// # Safety /// `handle` must be null or a pointer from [`ws_station_open_profile`] that has @@ -429,11 +412,10 @@ pub unsafe extern "C" fn ws_station_free(handle: *mut ws_station) { /// Panic on purpose, so the spike can measure what the guard does with it. /// -/// Behind a feature, and never in a shipped build. It exists because "we catch -/// panics" is a claim, and the spike's job is to turn claims into measurements: -/// with `panic = "abort"` anywhere in the profile that produced this library, -/// this function ends the calling C++ process instead of returning -/// [`WS_ERR_PANIC`], and no amount of `catch_unwind` in the source changes that. +/// Behind a feature, and never in a shipped build. Worth having because +/// `panic = "abort"` anywhere in the profile that produced this library makes +/// the call end the C++ process rather than return [`WS_ERR_PANIC`], and no +/// amount of `catch_unwind` in the source changes that. #[cfg(feature = "spike-probe")] #[no_mangle] pub extern "C" fn ws_debug_panic() -> c_int { @@ -469,11 +451,9 @@ pub type ws_log_callback = Option< /// The installed sink. Written once, read from aimdb's runtime thread. /// -/// A `static mut` behind a `OnceLock` rather than an `AtomicPtr` pair because -/// there is no way to *uninstall* it: `tracing`'s global subscriber is set for -/// the life of the process. That is the sharpest new constraint this door -/// found — see `README.md` — and the type reflects it rather than pretending a -/// second call could swap the pointer. +/// A `OnceLock` rather than an `AtomicPtr` pair because there is no way to +/// *uninstall* it: `tracing`'s global subscriber is set for the life of the +/// process. See `README.md`. struct Sink { callback: ws_log_callback, user_data: usize, @@ -523,11 +503,10 @@ fn c_level(level: &Level) -> c_int { /// A `tracing` layer that forwards events to a C function pointer. /// -/// The pendant of the Python door's `logging` bridge, and it exists for the -/// same reason: an FFI layer is a library inside somebody else's application, -/// and where the application's diagnostics go is the application's decision. No -/// aimdb library installs a subscriber; this one installs a *sink the caller -/// supplied*. +/// The pendant of the Python door's `logging` bridge, for the same reason: an +/// FFI layer is a library inside somebody else's application, and where the +/// diagnostics go is that application's decision. This installs no subscriber +/// of its own — only the sink the caller supplied. struct CLoggingLayer; impl Layer for CLoggingLayer { @@ -570,9 +549,8 @@ impl Layer for CLoggingLayer { /// Route the station's reporting — and aimdb's — to `callback`. /// /// Returns `true` if this call installed the sink, `false` if one was already -/// in place. Never panics and never aborts: a second call is something a -/// library-inside-a-library does all the time, and there is no exception type -/// here to carry the complaint. +/// in place. Never panics and never aborts: a second call is ordinary, and +/// there is no exception type here to carry a complaint. /// /// `filter` takes `tracing`'s `EnvFilter` syntax, defaults to `RUST_LOG` when /// `NULL`, and falls back to `info`. It is the cheap gate *below* the callback: diff --git a/weather-station-py/src/lib.rs b/weather-station-py/src/lib.rs index 801e638..3e3ef7f 100644 --- a/weather-station-py/src/lib.rs +++ b/weather-station-py/src/lib.rs @@ -1,27 +1,25 @@ //! The pyo3 door onto [`StationHandle`], built as a spike. //! //! Not the wheel: no maturin metadata, no build matrix, no distribution name. -//! It exists to find out what an FFI layer needs from the station crates -//! before they reach a registry. Findings are in `README.md`. +//! Findings are in `README.md`. //! //! # The GIL is the outermost lock //! -//! [`init_logging`] makes aimdb's runtime thread call into Python to log. From -//! that point on, every wait this module performs is part of a lock ordering, -//! and the rule is stronger than "release the GIL before joining a thread": +//! [`init_logging`] makes aimdb's runtime thread call into Python to log, so +//! from that point on every wait this module performs is part of a lock +//! ordering: //! //! > Never hold the GIL while acquiring anything the runtime thread can block //! > on. The GIL must be outermost. //! -//! Concretely: every method that can wait on the runtime thread — including -//! `close`, which joins it — wraps that wait in [`Python::detach`]. And -//! `StationHandle::is_closed` reads an atomic rather than the mutex `shutdown` -//! holds, so a getter called under the GIL can never block behind a shutdown -//! that is itself waiting for the GIL to be released. +//! Concretely: every method that can wait on the runtime thread — `close` +//! included, which joins it — wraps that wait in [`Python::detach`]. And +//! `StationHandle::is_closed` never takes the mutex `shutdown` holds, so a +//! getter called under the GIL cannot block behind a shutdown that is itself +//! waiting for the GIL. //! -//! None of this is visible in `StationHandle`'s signature — Rust has no GIL, so -//! no type can carry the constraint. It is written down here and exercised by -//! `python/spike.py`. +//! Rust has no GIL, so no signature can carry the constraint. It is written +//! down here and exercised by `python/spike.py`. use std::fmt::Write as _; use std::path::PathBuf; @@ -95,17 +93,12 @@ fn to_py_err(err: CoreStationError) -> PyErr { /// station.publish_temperature(21.5) /// ``` /// -/// `frozen` is what pyo3 offers for a class shared across threads, and this one -/// is: the supported shape is one reader thread per sensor publishing through a -/// single seat. It drops the runtime borrow flag a non-frozen pyclass carries, -/// which is what lets `close()` run while a publish is in flight — a `&mut -/// self` method cannot win an exclusive borrow from a `&self` method that is -/// parked in `Python::detach`, and fails with "Already borrowed" instead. -/// -/// The `Option` this class used to hold is gone with it: -/// `StationHandle::shutdown` takes `&self`, is idempotent, and tracks its own -/// closed flag, so the binding no longer re-solves any of that. The C ABI layer -/// gets the same three properties for free. +/// `frozen`, because the supported shape is one reader thread per sensor +/// publishing through a single seat. It drops the runtime borrow flag a +/// non-frozen pyclass carries, which is what lets `close()` run while a publish +/// is in flight — otherwise a `&mut self` method cannot win an exclusive borrow +/// from a `&self` method parked in `Python::detach`, and fails with "Already +/// borrowed". #[pyclass(frozen, name = "Station", module = "weather_station")] struct PyStation { inner: StationHandle, @@ -114,12 +107,10 @@ struct PyStation { impl PyStation { /// Refuse a call that needs a live runtime, with a message that says so. /// - /// Best-effort, and deliberately not a lock: `is_closed` reads an atomic, - /// so a close racing this check merely means the call fails one layer down - /// with aimdb's own "runtime thread has shut down" instead. This is about - /// the message, not about correctness — the producers refuse a publish - /// after close on their own, because dropping the handle releases the last - /// reference to the database their weak ones point at. + /// Best-effort, and deliberately not a lock: a close racing this check + /// merely means the call fails one layer down with aimdb's own "runtime + /// thread has shut down". This is about the message, not correctness — the + /// producers refuse a publish after close on their own. fn ensure_open(&self) -> PyResult<()> { if self.inner.is_closed() { return Err(StationError::new_err("this station is closed")); @@ -136,9 +127,7 @@ impl PyStation { /// releases the GIL for the duration. /// /// Takes `PathBuf` rather than `&str` so `pathlib.Path` and anything else - /// implementing `os.PathLike` work, which is what a Python caller reaches - /// for. Nothing changes on the Rust side: `StationHandle::open_profile` - /// already takes `impl AsRef`. + /// implementing `os.PathLike` work. #[staticmethod] fn open_profile(py: Python<'_>, path: PathBuf) -> PyResult { let handle = py @@ -181,10 +170,9 @@ impl PyStation { /// The slot number this station publishes into. /// - /// Still answers after `close()`. Deliberate: the slot and the name come - /// from the profile, not from the runtime, and a closed station is a thing - /// a log line or a traceback still wants to identify. Ask - /// [`closed`](Self::closed) for the state. + /// Still answers after `close()`: the slot and the name come from the + /// profile, not the runtime, and a closed station is still worth naming in + /// a traceback. Ask [`closed`](Self::closed) for the state. #[getter] fn slot(&self) -> u16 { self.inner.mesh_slot().slot() @@ -207,11 +195,11 @@ impl PyStation { /// Stop the station and shut its runtime thread down. Idempotent. /// /// A reading published in the last milliseconds before this does not - /// arrive — see `StationHandle::close`. + /// necessarily arrive — see `StationHandle::close`. /// /// `Python::detach` is required here, not optional: the shutdown joins /// aimdb's runtime thread, and after [`init_logging`] that thread needs the - /// GIL to log. Holding it across the join deadlocks. See the module docs. + /// GIL to log. Holding it across the join deadlocks. fn close(&self, py: Python<'_>) -> PyResult<()> { py.detach(|| self.inner.shutdown()).map_err(to_py_err) } @@ -271,12 +259,10 @@ impl MessageVisitor { /// A `tracing` layer that forwards events into Python's `logging`. /// -/// This is the whole reason the module no longer exports `init_tracing`. An -/// extension module is a library inside somebody else's application, and -/// process-wide logging is the application's decision. Forwarding gives aimdb's -/// events *more* control than a hardcoded stderr filter did, not less: levels -/// become a runtime question a Python operator answers with the tools they -/// already know. +/// Why the module exports no `init_tracing`: an extension module is a library +/// inside somebody else's application, and process-wide logging is that +/// application's decision. Forwarding makes levels a runtime question a Python +/// operator answers with the tools they already know. struct PyLoggingLayer; /// `logging` level numbers. `TRACE` has no Python equivalent, so it lands below @@ -345,9 +331,7 @@ impl Layer for PyLoggingLayer { /// `filter` is the cheap gate *below* Python: events it drops never acquire the /// GIL at all, which matters because the bridge runs on aimdb's runtime thread. /// It takes `tracing`'s `EnvFilter` syntax, defaults to `RUST_LOG`, and falls -/// back to `info` — the floor the stderr subscriber used to hardcode. Python's -/// own levels do the fine-grained work above it, so only lower this floor when -/// you actually want `debug` volume crossing the boundary. +/// back to `info`. Python's own levels do the fine-grained work above it. #[pyfunction] #[pyo3(signature = (filter = None))] fn init_logging(filter: Option<&str>) -> bool { diff --git a/weather-station/src/handle.rs b/weather-station/src/handle.rs index 3110f17..110d73a 100644 --- a/weather-station/src/handle.rs +++ b/weather-station/src/handle.rs @@ -18,23 +18,20 @@ use crate::{check_profile_version, AppProfile, BrokerProfile, MeshSlot, StationE /// How long [`StationHandle::open`] waits for the graph to start pumping. /// /// Generous: it covers building the graph and starting both connectors, not a -/// network round-trip. Exceeding it means the runtime thread is wedged, which -/// is worth an error rather than a station that publishes into nothing. +/// network round-trip. Exceeding it means the runtime thread is wedged. const GRAPH_START_TIMEOUT: Duration = Duration::from_secs(10); /// How long [`StationHandle::shutdown`] waits for the runtime thread to stop. /// -/// The same argument as [`GRAPH_START_TIMEOUT`], applied on the way out: a -/// wedged runtime thread should make shutdown fail, not hang the caller -/// forever. `AimDbHandle::detach` has no timeout of its own. +/// `AimDbHandle::detach` has no timeout of its own, and a wedged runtime thread +/// should fail the shutdown rather than hang the caller. const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(10); /// A joined mesh station driven from outside the async runtime. /// -/// The record graph runs on a background thread; this is the handle onto it. -/// Nothing about the mesh — the profile gate, the slot identity, the handshake, -/// the outbound links — is the caller's to get right, and no async appears in -/// the API, which is what makes this the type an FFI layer binds. +/// The record graph runs on a background thread; this is the handle onto it. No +/// async appears in the API, which is what makes this the type an FFI layer +/// binds. /// /// ```no_run /// # use std::thread; @@ -53,17 +50,14 @@ const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(10); /// # } /// ``` /// -/// The Rust templates use [`Station`](crate::Station) or -/// [`MeshSlot`](crate::MeshSlot) instead: they are already inside a Tokio -/// runtime, and this type would make them block a worker thread on a call the -/// graph could drive itself. +/// Callers already inside a Tokio runtime want [`Station`](crate::Station) or +/// [`MeshSlot`](crate::MeshSlot) instead; this type would block a worker thread. /// /// # Not reentrant into a runtime /// /// [`open`](Self::open) blocks on the broker pre-flight, so it must not be /// called from inside a Tokio runtime. That suits every FFI caller — a Python -/// or C station owns a plain OS thread — and it is why the async doors exist -/// for everyone else. +/// or C station owns a plain OS thread. pub struct StationHandle { slot: MeshSlot, temperature: SyncProducer, @@ -74,34 +68,23 @@ pub struct StationHandle { /// [`is_closed`](Self::is_closed) is what an FFI layer calls while holding /// its own interpreter lock. See the lock-ordering note on `shutdown`. closed: AtomicBool, - /// The fork generation this station was opened in. + /// In a mutex so [`shutdown`](Self::shutdown) can take `&self`: an FFI door + /// never receives `self` by value, and a `&mut self` door would collide + /// with a publish already in flight. /// - /// `fork` copies this struct but not the runtime thread, so a child holds a - /// station that can never publish again. Recorded here rather than asked of - /// `db` because [`is_closed`](Self::is_closed) must not take that mutex — - /// see the lock-ordering note on [`shutdown`](Self::shutdown). - made_in: aimdb_sync::fork::Generation, - /// In a mutex so [`shutdown`](Self::shutdown) can take `&self`: a - /// `#[pymethods]` method — and the C ABI's free function after it — never - /// receives `self` by value, and a `&mut self` door would collide with a - /// publish already in flight. - /// - /// A publish never contends for this lock: [`SyncProducer`] holds its own - /// `Weak` and reaches the database without going through the handle, - /// so `shutdown` can never queue behind one. - /// - /// Dropped last: the producers hold a weak reference to the database this - /// handle owns, so it has to outlive them. + /// A publish never contends for this lock — [`SyncProducer`] reaches the + /// database through its own `Weak` — so `shutdown` cannot queue + /// behind one. Dropped last: those weak references point at what this field + /// owns. db: Mutex>, } /// The mesh tables, parsed on behalf of a caller that has a file rather than a /// struct of its own. /// -/// The Rust doors take the tables already parsed, because a station composes -/// them into a profile naming its own extras. An FFI caller has no such struct, -/// so this door owns the parse — which is also the only way the profile gate -/// stays on the mesh's side of the boundary. +/// The Rust doors take the tables already parsed. An FFI caller has no such +/// struct, so this door owns the parse — which keeps the profile gate on the +/// mesh's side of the boundary. #[derive(Debug, Deserialize)] struct MeshProfile { profile_version: u64, @@ -159,9 +142,9 @@ impl StationHandle { // Registered after the slot's records, so it is the last future the // runner collects and therefore the last one polled in the first pass: // when it fires, both outbound links have subscribed to their buffers. - // Without this gate the first reading is lost roughly seven times in - // eight — `set()` still returns `Ok`, because a broadcast buffer - // accepts a value nobody is reading yet. + // Without this gate the first reading is usually lost — `set()` still + // returns `Ok`, because a broadcast buffer accepts a value nobody is + // reading yet. let (started_tx, started_rx) = mpsc::sync_channel::<()>(1); builder.on_start(move |_ctx| async move { let _ = started_tx.send(()); @@ -182,7 +165,6 @@ impl StationHandle { temperature, humidity, closed: AtomicBool::new(false), - made_in: aimdb_sync::fork::generation(), db: Mutex::new(Some(db)), }) } @@ -210,9 +192,8 @@ impl StationHandle { /// [`publish_temperature`](Self::publish_temperature) without blocking: /// fails rather than waiting when the outbound buffer is full. /// - /// The blocking form parks the calling thread, which for a Python caller - /// means parking the interpreter unless the binding releases the GIL around - /// it. This is the alternative where that does not fit. + /// The blocking form parks the calling thread — for a Python caller, the + /// interpreter, unless the binding releases the GIL around it. pub fn try_publish_temperature(&self, celsius: f32) -> Result<(), StationError> { self.temperature .try_set(TemperatureV2::new(celsius, unix_millis()?))?; @@ -235,29 +216,20 @@ impl StationHandle { /// Stop the station and shut the runtime thread down. /// - /// Idempotent, and safe to call while another thread is publishing: this + /// Idempotent, and safe to call while another thread is publishing: it /// takes `&self`, so no exclusive borrow has to be won from a publish - /// already in flight. That matters most for the shape that needs it — a - /// signal handler closing the station while its sensor threads run. + /// already in flight — the shape a signal handler needs. /// /// `publish_*` returns once the reading is in the buffer, not once it is on /// the wire, so a reading published in the last milliseconds before this - /// call may not arrive. How often is not fixed, which is the point: over - /// eight rounds of eight publish-then-close cycles against a loopback broker, - /// two to five of the eight temperatures arrived and none to four of the - /// humidities — the second of the two publishes has less time and fares - /// worse. That is accepted rather than papered over: stations are - /// long-lived and publish on a cadence, so the reading lost to a shutdown - /// is one nobody would have read. A station that publishes once and exits - /// needs a delivery signal — an ACK topic — not a close that waits, since - /// no wait makes delivery certain. + /// call may not arrive. That is accepted rather than papered over: stations + /// publish on a cadence, and no wait makes delivery certain. A station that + /// publishes once and exits needs a delivery signal — an ACK topic. /// /// After this returns, `publish_*` fails with - /// [`SyncError::RuntimeShutdown`](aimdb_sync::SyncError::RuntimeShutdown): - /// dropping the handle releases the last `Arc` to the database, so the - /// producers' weak references stop upgrading. That is deliberate. Keeping - /// the handle alive would let `set()` go on pushing into a buffer nobody - /// reads and go on returning `Ok`, which loses readings silently. + /// [`SyncError::RuntimeShutdown`](aimdb_sync::SyncError::RuntimeShutdown), + /// deliberately: keeping the database alive would let `set()` go on + /// returning `Ok` into a buffer nobody reads. /// /// # Lock ordering /// @@ -265,8 +237,8 @@ impl StationHandle { /// `let` below rather than matching on the `take()` directly, which would /// extend the guard's lifetime to the end of the match. A caller that /// blocks on this mutex while holding a lock the runtime thread needs (an - /// FFI layer's interpreter lock, say, when that thread logs through a - /// bridge into it) would otherwise deadlock against its own shutdown. + /// FFI layer's interpreter lock, when that thread logs through a bridge into + /// it) would otherwise deadlock against its own shutdown. pub fn shutdown(&self) -> Result<(), StationError> { let taken = self .db @@ -286,16 +258,21 @@ impl StationHandle { /// Whether this station can still publish. /// - /// True after [`shutdown`](Self::shutdown), and true in a process that has - /// `fork`ed since the station was opened — a child inherits the struct but - /// not the runtime thread, so its station is closed in every sense that - /// matters to a caller deciding whether to publish. + /// True after [`shutdown`](Self::shutdown), and true whenever a publish + /// could no longer reach the graph — the runtime thread is gone, or this + /// process `fork`ed since the station was opened and the thread did not + /// come across. + /// + /// The second half is asked of the producer rather than tracked here, so it + /// is the very check a publish goes through: this cannot report open while + /// a publish would be refused. /// - /// Reads two atomics, never the mutex: a caller holding an interpreter lock - /// can ask this while a shutdown is joining the runtime thread without - /// closing the cycle described on `shutdown`. + /// Never takes the mutex — the producer is reachable without it — so a + /// caller holding an interpreter lock can ask this while a shutdown is + /// joining the runtime thread, without closing the cycle described on + /// `shutdown`. pub fn is_closed(&self) -> bool { - self.closed.load(Ordering::Acquire) || aimdb_sync::fork::forked_since(self.made_in) + self.closed.load(Ordering::Acquire) || self.temperature.check().is_err() } /// [`shutdown`](Self::shutdown) for a caller that owns the handle by value. @@ -310,8 +287,7 @@ impl StationHandle { /// Wall-clock milliseconds. /// /// A reading with no usable timestamp is worse than no reading: the hub keys -/// its dew-point join off them, so a station whose clock is unset would poison -/// its slot rather than merely go quiet. +/// its dew-point join off them. fn unix_millis() -> Result { SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH)