Skip to content

Fix WebRTC callback shutdown deadlock - #2778

Open
voropaevv wants to merge 1 commit into
roboflow:mainfrom
voropaevv:codex/fix-webrtc-callback-close-deadlock
Open

Fix WebRTC callback shutdown deadlock#2778
voropaevv wants to merge 1 commit into
roboflow:mainfrom
voropaevv:codex/fix-webrtc-callback-close-deadlock

Conversation

@voropaevv

Copy link
Copy Markdown
Contributor

Why

WebRTCSession.close() may be called by on_data and on_error callbacks on the session event-loop thread. It previously submitted cleanup coroutines to that same loop and blocked on Future.result(), preventing the loop from running them. The public state changed to CLOSED before teardown, so a later close() call could not recover the pending cleanup.

Fixes #2777

What changed

  • dispatch event-loop initiated cleanup to one daemon helper without blocking the loop
  • serialize cleanup ownership so peer, source, and loop resources are torn down once
  • keep external and context-manager shutdown synchronous by waiting for cleanup completion
  • preserve source and loop cleanup when peer shutdown fails, and allow retry if the helper thread cannot start
  • add deterministic real-loop lifecycle tests for callback-first, startup-lock, external-first, context-manager, duplicate-close, and failure schedules

Verification

  • baseline regression test fails on the unmodified parent because the event-loop callback never returns
  • .venv/bin/python -m pytest tests/inference_sdk/unit_tests/webrtc/test_session_lifecycle.py -q — 32 passed
  • .venv/bin/python -m pytest tests/inference_sdk/unit_tests/webrtc -q — 83 passed
  • .venv/bin/python -m pytest tests/inference_sdk/unit_tests -q — 519 passed, 86 existing deprecation warnings
  • lifecycle coverage run exercises every executable line added to the shutdown implementation
  • PATH="$PWD/.venv/bin:$PATH" make check_code_quality — passed
  • git diff --check — passed

Not verified

  • no live remote WebRTC server or browser session was used; the regression is thread/loop lifecycle logic covered with a real background asyncio loop and async fake peer/source objects
  • cleanup of a peer connection constructed before _init() assigns self._pc is separate pre-existing startup-failure behavior and remains outside this change

Risks / rollback

The change is isolated to the SDK close lifecycle. External close() remains synchronous; only calls originating on the internal event loop are dispatched. Reverting commit 7da042d9e08d30a367e505e89ba91a6429fad55f restores the previous behavior.

Migrations / external effects

None.

Screenshots / preview

Not applicable; no UI changes.

Dispatch event-loop initiated cleanup to a helper thread while keeping external close calls synchronous. Add deterministic lifecycle coverage for callback, startup-lock, concurrent, and failure schedules.

Fixes roboflow#2777
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.

WebRTCSession.close() deadlocks in data-channel callbacks

1 participant