fix(deps): stop mio 0.6.23's null-deref UB check aborting Windows tests - #3092
Merged
Conversation
Contributor
|
😎 Merged successfully - details. |
mio 0.6.23 computes field offsets with the pre-`offset_of!` idiom `&(*(0 as *const T)).field as *const _ as usize`. Its Windows IOCP completion handlers run that on every UDP/TCP completion, so with debug assertions on, rustc's null-pointer check fires a non-unwinding panic and aborts the process. That killed the `dora-ros2-bridge` test binary in the Windows nightly as soon as `context_new_dds_uses_requested_domain` brought up a RustDDS participant. mio 0.6 cannot be upgraded away: `mio_06 = "^0.6.23"` is a non-optional direct dependency of every published rustdds (0.14.0 included) and of ros2-client 0.8-0.10, so the #2854 port does not remove it. Opt just that dependency out of debug assertions instead; release builds already behave this way. Fixes #3091 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
phil-opp
force-pushed
the
fix-3091-mio-nullcheck
branch
from
August 9, 2026 11:54
40d80fc to
f8d3c05
Compare
Collaborator
Author
|
🤖 Fully automated review by Claude. No human has verified these findings — treat them as advisory. No issues found. The Generated by Claude Code |
phil-opp
added a commit
that referenced
this pull request
Aug 11, 2026
Bumps `dora-ros2-bridge` from ros2-client 0.8.0 / rustdds =0.11.4 to ros2-client 0.10.1 / rustdds 0.14, lifting the `=` pin on RustDDS. The two are one upgrade: ros2-client 0.10.1 declares `rustdds = "0.14"`. The pin existed because rustdds 0.11.5-0.13.x depended unconditionally on `pnet`, whose `pnet_datalink` links `Packet.lib` on Windows (Atostek/RustDDS#375). RustDDS 0.14 replaced `pnet` with `netdev`, which uses `windows-sys` and needs no npcap SDK, so the pin lifts with no Windows workaround: `cargo tree -e normal --workspace --target x86_64-pc-windows-msvc` resolves zero `pnet*` crates, and nightly keeps building and testing the bridge there. That matters beyond tidiness -- excluding the bridge from Windows would have cancelled out #3092, the mio 0.6.23 debug-assertions fix that stopped its test binary aborting. No source changes were required. Every ros2-client symbol the bridge uses survives 0.8 -> 0.10.1 (the public-surface diff is additions only), and the removed APIs -- `QosPolicyBuilder::property()` and the `rustdds::CdrEncodingSize` re-export -- were never referenced here. RustDDS 0.14 additionally gates its mio-0.8 integration behind an opt-in feature that nothing here uses. `ros2-client` and `cdr-encoding` move to `[workspace.dependencies]` so the bridge and its arrow helper cannot drift apart on the ROS distro. `cdr-encoding` goes 0.10.2 -> 0.11 to match what RustDDS depends on, leaving a single copy in the lockfile. One consequence needs review attention: the ROS distro is now explicit, and this picks `humble`. 0.10 gained distro features (default `jazzy`); 0.8 only had a `pre-iron-gid` opt-out. `humble` selects a 24-byte `Gid` where the current build emits 16, changing `rmw_dds_common` graph-discovery wire compatibility -- deliberately, to match the distro dora's own ROS2 harness runs. Topic, service, and action payloads are ordinary CDR and interoperate either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
phil-opp
added a commit
that referenced
this pull request
Aug 11, 2026
Rebases the port onto the upstream fix. When the previous commit was written, ros2-client 0.10.0 required rustdds ^0.13, and every rustdds in that range depended unconditionally on `pnet`, whose `pnet_datalink` links `Packet.lib` on Windows (Atostek/RustDDS#375) -- hence the CI exclusion. RustDDS 92c9b117c replaced `pnet` with `netdev`, which uses `windows-sys` and needs no npcap SDK. That shipped in rustdds 0.14.0, #375 is closed, and ros2-client 0.10.1 followed with `rustdds ^0.14`. So: - ros2-client 0.10.0 -> 0.10.1, rustdds 0.13.1 -> 0.14 - the `ros2_excludes` matrix variable and its four `--exclude` flags go away, so nightly builds and tests the bridge on Windows again - the Windows caveat leaves docs/ros2-bridge.md, keeping only the distro note Restoring that coverage matters beyond tidiness: the exclusion would have cancelled out #3092, the mio 0.6.23 debug-assertions fix that stopped the bridge test binary aborting on Windows. mio 0.6.23 is still a non-optional transitive dependency under rustdds 0.14, so that workaround stays. Verified: `cargo tree -e normal --workspace --target x86_64-pc-windows-msvc` resolves zero `pnet*` crates; `cargo check --target x86_64-pc-windows-gnu` on the bridge and bridge node is clean; fmt, clippy -D warnings on the bridge crates, and `cargo check --examples` are clean; `cargo deny check licenses advisories bans sources` all ok; workspace tests 1854 passed / 2 failed, both pre-existing no-multicast rmw_zenoh cases that fail identically on main. `Cargo.lock` shrinks 70 lines net. Not verified: an actual Windows link -- no Windows host available, and `cargo check` does not link. A nightly run is the confirmation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
trunk-io Bot
pushed a commit
that referenced
this pull request
Aug 11, 2026
* feat(ros2-bridge): port to ros2-client 0.10.0 and RustDDS 0.13.1 Bumps `dora-ros2-bridge` from ros2-client 0.8.0 / rustdds =0.11.4 to ros2-client 0.10.0 / rustdds 0.13.1, and lifts the `=` pin on RustDDS. The two are one upgrade: ros2-client 0.10 declares `rustdds = "0.13"`. No source changes were required. Every ros2-client symbol the bridge uses survives 0.8 -> 0.10 (the public-surface diff is additions only), and the removed APIs — `QosPolicyBuilder::property()` and the `rustdds::CdrEncodingSize` re-export — were never referenced here. `ros2-client` and `cdr-encoding` move to `[workspace.dependencies]` so the bridge and its arrow helper cannot drift apart on the ROS distro. `cdr-encoding` goes 0.10.2 -> 0.11 to match what RustDDS 0.13 depends on, leaving a single copy in the lockfile; its only API change tightens `from_bytes` to `&'de [u8]`, and every call site here yields owned types. Two consequences worth review attention: ROS distro is now explicit. 0.10 gained distro features (0.8 only had a `pre-iron-gid` opt-out), and they select `Gid` width. This builds with `humble`, giving a 24-byte `Gid` where the previous build had 16 — a deliberate wire change that aligns the build with the distro our harness actually runs (`scripts/ros2dev.sh`). It affects `rmw_dds_common` graph discovery only; topic/service/action payloads are untouched. Windows is excluded rather than fixed. RustDDS 0.11.5+ depends unconditionally on `pnet`, whose `pnet_datalink` declares `#[link(name = "Packet")]` on Windows, so linking needs the Npcap SDK (Atostek/RustDDS#375, still open — this was the reason for the pin). The four crates that transitively pull RustDDS are excluded from the Windows build/test steps in nightly.yml. `Packet.lib` is link-time only, so the `cargo check` steps and the `cross-check` job's windows-gnu target are unaffected and stay as they were. The durable fix is upstreaming an optional-`pnet` feature — RustDDS needs it for one `pnet::datalink::interfaces()` call. Verified on Linux against a real ROS 2 Humble install: fmt, clippy -D warnings, `cargo check --examples`, 110 bridge tests, 1543 workspace tests, and `cargo deny check licenses advisories bans sources` all pass. Live interop against `demo_nodes_cpp talker` receives messages and shows the dora node in `ros2 node list` with a 24-octet GID; the same probe built with the `jazzy` default is absent from `ros2 node list` while still receiving payloads, confirming the Gid width governs graph discovery only. The Windows link failure and the exclusion that avoids it are NOT verified here — no Windows host was available. That needs a nightly run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(ros2-bridge): take RustDDS 0.14 and drop the Windows exclusion Rebases the port onto the upstream fix. When the previous commit was written, ros2-client 0.10.0 required rustdds ^0.13, and every rustdds in that range depended unconditionally on `pnet`, whose `pnet_datalink` links `Packet.lib` on Windows (Atostek/RustDDS#375) -- hence the CI exclusion. RustDDS 92c9b117c replaced `pnet` with `netdev`, which uses `windows-sys` and needs no npcap SDK. That shipped in rustdds 0.14.0, #375 is closed, and ros2-client 0.10.1 followed with `rustdds ^0.14`. So: - ros2-client 0.10.0 -> 0.10.1, rustdds 0.13.1 -> 0.14 - the `ros2_excludes` matrix variable and its four `--exclude` flags go away, so nightly builds and tests the bridge on Windows again - the Windows caveat leaves docs/ros2-bridge.md, keeping only the distro note Restoring that coverage matters beyond tidiness: the exclusion would have cancelled out #3092, the mio 0.6.23 debug-assertions fix that stopped the bridge test binary aborting on Windows. mio 0.6.23 is still a non-optional transitive dependency under rustdds 0.14, so that workaround stays. Verified: `cargo tree -e normal --workspace --target x86_64-pc-windows-msvc` resolves zero `pnet*` crates; `cargo check --target x86_64-pc-windows-gnu` on the bridge and bridge node is clean; fmt, clippy -D warnings on the bridge crates, and `cargo check --examples` are clean; `cargo deny check licenses advisories bans sources` all ok; workspace tests 1854 passed / 2 failed, both pre-existing no-multicast rmw_zenoh cases that fail identically on main. `Cargo.lock` shrinks 70 lines net. Not verified: an actual Windows link -- no Windows host available, and `cargo check` does not link. A nightly run is the confirmation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: bdingfd <bding@futuredial.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Philipp Oppermann <dev@phil-opp.com>
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.
mio 0.6.23(reached viamio-extras→rustdds/ros2-client) computes structfield offsets with the pre-
offset_of!idiom:That is a literal null-pointer dereference. Its Windows IOCP completion handlers
(
overlapped2arc!, 6 call sites acrosssys/windows/udp.rsandtcp.rs) run it onevery completed UDP/TCP operation, so with
debug-assertionson, rustc'snull-pointer check fires a non-unwinding panic — uncatchable, aborts the process:
In the Windows nightly this lands the moment
transport::tests::context_new_dds_uses_requested_domainbrings up a real DDSparticipant, taking the rest of the
dora-ros2-bridgelib tests with it. It alsomeans any debug build that actually receives DDS traffic on Windows aborts —
release builds are unaffected because
debug-assertionsis off there.Not a regression from a dependency bump, and not new: it has been latent since that
test was added on 07-21. The 08-03/08-04 nightlies failed earlier, at
cargo check --all, on unrelated Windows-only compile errors; once those were fixedthe job got far enough to reach this.
Why not just upgrade
Checked every published version in the crates.io index — mio 0.6 cannot be upgraded
away:
mio 0.6.23(2019) is the last 0.6.x.mio_06 = "^0.6.23"is a non-optional direct dependency of every publishedrustdds, 0.14.0 included.ros2-client0.8.1 / 0.9.0 / 0.10.0 each depend onmio ^0.6.23directly.mio-extras 2.0.6requiresmio ^0.6.14on top.So the
ros2-client0.10 / RustDDS 0.13.1 port in #2854 would not have fixed this.The only way to remove the UB rather than the check is to patch mio itself — a
one-line
offset_of!→::std::mem::offset_of!change, which I verified drops theinserted assertions in mio's Windows MIR from 57 to 0. It has to be a git source:
a vendored path patch loses
--cap-lints allowand mio 0.6 then fails to build onmodern rustc. That was weighed and deferred — a forked git dependency is more
supply-chain surface (cargo-deny / audit / license jobs, and a divergence from what
crates.io consumers resolve) than this nightly failure justifies.
What this does
Opts the one dependency out of debug assertions, restoring the behavior mio 0.6 was
written against. The version is pinned in the profile spec on purpose: if
mio 0.6.23ever does leave the graph,cargo checkwarns that the spec matchesnothing, which is the cue to delete the block.
Verified on rustc 1.97.1 (the pinned CI toolchain) with a minimal two-crate
reproduction of mio's macro: debug build aborts with the identical message, the
per-package override makes it pass, release was always fine. Confirmed in this
workspace that the spec matches
mio v0.6.23and notmio0.8.11/1.2.2, and that awrong version warns.
Known limitation: workspace profile overrides don't reach downstream consumers, so
someone depending on
dora-ros2-bridgefrom crates.io and building in debug onWindows still hits the abort. Fixing that needs an upstream RustDDS change.
PR CI is Linux-only, so the real confirmation is the next Windows nightly.
Fixes #3091