fix(ros2-bridge): don't panic on zero-field message types (std_msgs/Empty) - #2864
Draft
phil-opp wants to merge 1 commit into
Draft
fix(ros2-bridge): don't panic on zero-field message types (std_msgs/Empty)#2864phil-opp wants to merge 1 commit into
phil-opp wants to merge 1 commit into
Conversation
…mpty) Deserializing a ROS 2 message type with zero fields (e.g. `std_msgs/msg/Empty`) panicked in the CDR->Arrow conversion because `StructArray::from(Vec<..>)` panics on an empty field vec — the array length is ambiguous with no columns. Because the deserializer runs inside the subscription take path while a RustDDS cache lock is held, that panic poisoned the DDS cache mutex and subsequently killed the participant event loop, silently taking down every subscription on the participant (0 Hz on all topics) after a single stray Empty message. Guard both the deserialize path (`deserialize/mod.rs`) and the default value path (`serialize/defaults.rs`) to build a length-1 empty-fields struct via `StructArray::new_empty_fields(1, None)` when there are no fields, matching the pattern already used in the defaults test code. Adds a regression test that deserializes a zero-field message. Fixes #2804 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpY25UTvkyfgyTaJYcAQ7v
Contributor
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Collaborator
Author
|
Automated review by Claude — this is a fully automated review; no human has vetted it. No issues found. Generated by Claude Code |
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
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.
Summary
Fixes #2804.
Deserializing a ROS 2 message type with zero fields (e.g.
std_msgs/msg/Empty) panicked in the ros2-bridge CDR→Arrow conversion:StructArray::from(Vec<..>)panics on an empty field vec because the array length is ambiguous with no columns. Since the deserializer runs inside the subscription take path while a RustDDS cache lock is held, the panic poisoned the DDS cache mutex and then killed the participant event loop — silently taking down every subscription on the participant (0 Hz on all topics) after a single strayEmptymessage.Changes
deserialize/mod.rs(the take path) andserialize/defaults.rs(default-value construction) now build a length‑1 empty‑fields struct viaStructArray::new_empty_fields(1, None)when there are no fields, instead offields.into(). This mirrors the pattern already used in the crate's default-value test code.empty_message_deserializesthat deserializes a zero‑field message and asserts it produces a single struct row with no columns.Testing
cargo test -p dora-ros2-bridge-arrow— 16 passed, incl. the new regression testcargo clippy -p dora-ros2-bridge-arrow -- -D warnings— cleancargo fmt --all -- --check— cleanGenerated by Claude Code