Skip to content

Commit 12a968a

Browse files
committed
fix(node): bound the FFI advanced dispatch queue too
The conversion to dispatch_capacity covered three of four CallbackDispatcher::spawn sites. node.rs's advanced (TransientLocal) arm still passed DISPATCH_UNBOUNDED, so an FFI raw subscriber declaring KeepLast(n) got an unbounded queue fed by remote samples -- the exact defect the other three sites were changed to remove. Two shipped doc comments and the PR description asserted 'both paths pass dispatch_capacity'. There are four paths, and one did not. Nothing caught it because the ffi feature is enabled by no crate, so this arm is never compiled on the PR gate (#291). Found by an audit agent reading the diff against its own description.
1 parent 0d48f84 commit 12a968a

2 files changed

Lines changed: 15 additions & 6 deletions

File tree

crates/hiroz/src/node.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ impl ZNode {
642642
use crate::{
643643
entity::{EndpointEntity, EndpointKind},
644644
pubsub::{
645-
CallbackDispatcher, DISPATCH_UNBOUNDED, SubscriberHandle,
645+
CallbackDispatcher, SubscriberHandle,
646646
apply_transient_local_sub, dispatch_capacity, qos_needs_advanced,
647647
},
648648
topic_name,
@@ -678,8 +678,11 @@ impl ZNode {
678678
// never runs on a thread that is inside a hiroz publish — see
679679
// `pubsub::CallbackDispatcher`.
680680
let subscriber = if qos_needs_advanced(&entity.qos) {
681-
let dispatcher =
682-
CallbackDispatcher::spawn(&qualified_topic, raw_callback, DISPATCH_UNBOUNDED)?;
681+
let dispatcher = CallbackDispatcher::spawn(
682+
&qualified_topic,
683+
raw_callback,
684+
dispatch_capacity(&entity.qos),
685+
)?;
683686
let subscriber = self
684687
.session
685688
.declare_subscriber((*topic_ke).clone())

crates/hiroz/src/pubsub.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -392,9 +392,15 @@ impl CallbackDispatcher {
392392
/// [`Self::local_only_shim`] to obtain the callback to hand to zenoh.
393393
///
394394
/// `capacity` is the number of undelivered samples retained before the
395-
/// oldest is dropped. Both paths pass [`dispatch_capacity`], so a callback
396-
/// subscriber retains what its history QoS declares regardless of which
397-
/// path it takes. See the "Backpressure" section.
395+
/// oldest is dropped. **All four construction sites pass
396+
/// [`dispatch_capacity`]** — the plain and advanced arms of both the typed
397+
/// builder (this module) and the FFI raw subscriber (`node.rs`) — so a
398+
/// callback subscriber retains what its history QoS declares regardless of
399+
/// which path it takes. See the "Backpressure" section.
400+
///
401+
/// The FFI advanced arm was missed when the other three were converted, and
402+
/// nothing caught it: the `ffi` feature is enabled by no crate, so that arm
403+
/// is not compiled on the PR gate at all (#291).
398404
pub(crate) fn spawn<F>(topic: &str, handler: Arc<F>, capacity: usize) -> Result<Self>
399405
where
400406
F: Fn(Sample) + Send + Sync + 'static,

0 commit comments

Comments
 (0)