Skip to content

fix(ipc): drop stale ring on failed re-registration - #261

Open
WilliamK112 wants to merge 1 commit into
NVIDIA:mainfrom
WilliamK112:codex/fix-ipc-reregister-ring-failure
Open

fix(ipc): drop stale ring on failed re-registration#261
WilliamK112 wants to merge 1 commit into
NVIDIA:mainfrom
WilliamK112:codex/fix-ipc-reregister-ring-failure

Conversation

@WilliamK112

@WilliamK112 WilliamK112 commented Jun 23, 2026

Copy link
Copy Markdown

Summary

  • remove the stale connector ring entry before closing the old ring during re-registration
  • leave the connector without a registered ring if opening the replacement shared-memory segment fails, instead of retaining a closed ring in _ring_registry
  • add a regression test for the failed-reopen path that does not depend on an IPC socket fixture

Fixes #209.

Validation

Rebased onto current NVIDIA/xr-ai@37822349729f9c67f492f9aac29b7eeb803d1dd9 on 2026-08-09. The only conflict was the upstream SDK rename from xr_ai_agent to xr_ai_hub; the refreshed test uses the current package.

  • focused failed-reopen regression: 1 passed
  • complete tests/test_participant_events.py: 5 passed
    • run with a short TMPDIR on macOS to stay below the Unix-domain socket path limit
  • Ruff 0.15.16 on both changed Python files: passed
  • repository SPDX checker on both changed files: passed
  • DCO sign-off present
  • git diff --check upstream/main...HEAD: passed

@WilliamK112
WilliamK112 force-pushed the codex/fix-ipc-reregister-ring-failure branch from cf1dfb1 to f64d414 Compare July 2, 2026 02:59
@WilliamK112
WilliamK112 marked this pull request as ready for review July 2, 2026 02:59
@WilliamK112
WilliamK112 force-pushed the codex/fix-ipc-reregister-ring-failure branch from f64d414 to e6f8d5f Compare August 9, 2026 12:00
@WilliamK112

Copy link
Copy Markdown
Author

Refreshed this PR onto current main (37822349) at head e6f8d5f4. The only conflict was the upstream SDK rename, so the regression test now imports ConnectorRegistration from xr_ai_hub. Current validation: all 5 participant-event tests pass with a short macOS TMPDIR; focused regression, Ruff 0.15.16, SPDX, DCO, and diff checks are clean.

@WilliamK112
WilliamK112 force-pushed the codex/fix-ipc-reregister-ring-failure branch from e6f8d5f to ed61f3f Compare August 12, 2026 05:27
@WilliamK112

Copy link
Copy Markdown
Author

Refreshed again onto current main (6de6ca73) after XR-Media-Hub moved from server-runtime/ to services/. Git followed the rename cleanly; the PR remains one signed commit and the diff is still limited to the registry pop plus its regression test. GitHub now reports behind_by: 0, ahead_by: 1, and mergeable. Validation on head ed61f3f2: all 5 test_participant_events.py tests pass, Ruff 0.15.16 passes, SPDX and DCO checks pass, and git diff --check is clean.

Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
@WilliamK112
WilliamK112 force-pushed the codex/fix-ipc-reregister-ring-failure branch from ed61f3f to 13cb713 Compare August 13, 2026 18:26
@WilliamK112

Copy link
Copy Markdown
Author

Refreshed onto current main (b641bf5) at signed head 13cb7135. The six upstream commits replayed cleanly, including the concurrent xr-ai-hub-client changes; the PR diff remains limited to the stale-ring pop and its regression test. Validation: all 5 test_participant_events.py tests pass, Ruff passes, both changed files pass the SPDX checker, the commit retains its DCO sign-off, and git diff --check is clean. GitHub reports the PR mergeable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IPC hub: connector re-register left bound to closed ring if ShmRingBuffer reopen fails

1 participant