Skip to content

Two event-API sharp edges: set_callback fires the backlog under the caller's lock; duplicate register + one unregister drops all #289

Description

@YuanYuYuan

Part of #282. Two public-API sharp edges found in the same adversarial pass. Both pre-existing; #260 makes the second more visible by promoting it to a documented trait API.

1. EventsManager::set_callback is still pub, and is still the failure-3 footgun

It takes &mut self, so a caller holding an Arc<Mutex<EventsManager>> can only reach it with the outer mutex held — and it fires the backlog underneath that guard. A callback that re-enters (rmw_take_eventRmEventHandle::take_event) then self-deadlocks on a non-reentrant Mutex.

That is #259's failure 3 verbatim, still reachable from shipped public API. Its own doc comment says so, which helps a reader but prevents nothing.

The tripwire cannot catch it either: Mutex<EventsManager> is caller-owned and untracked.

  • #[deprecated] with a pointer to the collect-release-fire shape, or make it pub(crate)
  • If it must stay public, route the backlog fire through invoke_user_callback! so a violation is at least detected in debug

2. Duplicate registration + single unregistration removes every entry

register_graph_guard_condition pushes with no dedup. unregister_graph_guard_condition uses retain(|gc| !Arc::ptr_eq(..)), which removes all matching entries.

register(gc);
register(gc);
unregister(&gc);   // leaves ZERO registrations, not one

Pre-existing shape — the previous usize-keyed version had the same retain — but #260 promotes this to a documented public trait API taking Arc<dyn GraphGuardCondition>, and its new unit test exercises only the 1:1 case.

Unregistering a never-registered handle is a silent no-op, also untested.

  • Decide the contract: reject duplicate registration, or make unregister remove exactly one
  • Test both degenerate cases

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions