Possible error in pubsub example: Subscriber class looses ref to node

[MaxiMaerz]

I was trying to create a subscriber and used your rust_pubsub example as a guide. However, I think there is a mistake:

What happens

The pubsub example exists in the first call of executor.spin(SpinOptions::default()).first_error()

Why i think this happens

The SimpleSubscriptionNode takes the executor as an input but does not store it as an attribute. When the constructor returns, it will lose the reference to the executor. When this happens, the executor drops the weakref to the node resulting in spinning nothing:

            if self.nodes_mtx.lock().unwrap().is_empty() {
                // Nothing to spin for, so just quit here
                return Vec::new();
            }

Quickfix

Assign the node to the subscriber

pub struct SimpleSubscriptionNode {
    _subscriber: Arc<Subscription<StringMsg>>,
    data: Arc<Mutex<Option<StringMsg>>>,
    node: Arc<Node>,
}

impl SimpleSubscriptionNode {
    fn new(executor: &Executor) -> Result<Self, RclrsError> {
        let node = executor.create_node("simple_subscription").unwrap();
        let data: Arc<Mutex<Option<StringMsg>>> = Arc::new(Mutex::new(None));
        let data_mut: Arc<Mutex<Option<StringMsg>>> = Arc::clone(&data);
        let _subscriber = node
            .create_subscription::<StringMsg, _>(
                "publish_hello",
                QOS_PROFILE_DEFAULT,
                move |msg: StringMsg| {
                    *data_mut.lock().unwrap() = Some(msg);
                },
            )
            .unwrap();
        Ok(Self { _subscriber, data, node })
    }

I'm a bit occupied with my project, otherwise I would submit a PR. But it could also make sense to raise a warning if the executor has no references left.

[mxgrey]

Indeed, this example is caught in the middle of a transition right now. It was working before #428 (by explicitly holding the node as you're doing here), and it'll work again after #446, but since #446 was broken down into a series of separate PRs, I overlooked that this example doesn't work until the PR series is finished.

Rather than add a field to the struct to explicitly keep the node alive, I'd like to support the RAII relationship between Subscription and Node as soon as possible. I'll open a minimal PR to make the example work as-is, and I'll add a test case that catches this usage pattern.

[MaxiMaerz]

Just tested the fix, it works for me. Thank you for the fast fix!