Open
Conversation
YuanYuYuan
changed the title
Three related fixes to rmw_wait and the notifier:
1. Notifier::notify_all() now holds the mutex before calling cv.notify_all().
This closes the lost-wakeup race: a notification fired between check_ready()
and wait_for() would previously be silently dropped.
2. WaitSetImpl::wait() switches from duration-based to deadline-based waiting.
The absolute deadline is computed once at entry; each loop iteration uses
the remaining time. This prevents spurious wakeups from extending the total
wait beyond the requested timeout.
3. rmw_wait() now NULLs out all entity arrays on RMW_RET_TIMEOUT.
The RMW contract requires that entries which are not ready be set to NULL.
Previously the TIMEOUT path returned without touching the arrays, leaving
stale non-NULL pointers in place. rcl/wait.c reads these back
unconditionally (is_ready = guard_conditions[i] != NULL), so rcl would
see every guard condition as "triggered". When rcl then returned
RCL_RET_OK because a timer had fired, ThreadSafeSynchronization::sync_wait
interpreted the stale interrupt guard condition pointer as an internal
class interrupt (was_interrupted_by_this_class=true) and looped forever
instead of returning WaitResultKind::Ready — causing all timer-based
tests to time out.
Fixes: test_thread_safe_synchronization/wait_timer,
test_executors/StaticSingleThreadedExecutor.spinWithTimer,
test_executors/StaticSingleThreadedExecutor.spinWhileAlreadySpinning,
test_executor/spin_all_fail_wait_set_clear,
test_dynamic_storage, test_static_storage, test_logger_service
The rmw_event_type_to_zenoh_event() mapping assumed RMW_EVENT_INVALID=0 at the start of the enum, but rmw/event.h starts with RMW_EVENT_LIVELINESS_CHANGED=0 and RMW_EVENT_INVALID=11 is the sentinel at the end. This caused every event type to be mapped to the wrong ZenohEventType, preventing QoS incompatibility callbacks and other event-driven features from working. Fixes test_events_executor/test_default_incompatible_qos_callbacks and improves rclcpp test pass rate from 755/781 to 765/791.
…counts GraphEventManager.trigger_graph_change was broadcasting matched events to ALL registered entities regardless of topic, causing double-counting and negative current_count values. Additionally, remove_local_entity and the liveliness DELETE callback both fired trigger_graph_change, producing duplicate events. Changes: - Store topic per entity in GraphEventManager for topic-based filtering - Filter trigger_graph_change to only notify entities on the same topic - Remove duplicate trigger_graph_change from remove_local_entity (liveliness DELETE callback already handles it) - Clamp current_count to >= 0 in EventsManager::update_event_status - Use .max(0) when casting i32 -> usize in rmw_take_event to prevent negative values becoming u64::MAX
- Add input validation to all RMW graph functions: - Node name/namespace validation via rmw_validate_node_name/namespace - Topic/service name validation via rmw_validate_full_topic_name - Allocator validation via rcutils_allocator_is_valid - Pre-initialized array checks (rmw_names_and_types_check_zero, etc.) - Replace HashMap with BTreeMap in graph API for deterministic ordering - Fix serialization buffer: truncate to actual CDR size instead of sending the full pre-allocated buffer (was always >=512 bytes) - Achieves 14/16 test_rmw_implementation (matches rmw_zenoh_cpp) - All ros2cli tests pass (ros2node, ros2service, ros2action, ros2interface, ros2topic)
YuanYuYuan
force-pushed
the
dev/rclcpp
branch
from
3fd4ed6 to
8e336a5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.