Skip to content

fix(rtc): drain publisher negotiation during session shutdown - #5

Merged
tiredIsa merged 1 commit into
native-v2from
feat/126-negotiation-shutdown
Sep 6, 2026
Merged

tiredIsa merged 1 commit into
native-v2from
feat/126-negotiation-shutdown

Conversation

@tiredIsa

@tiredIsa tiredIsa commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Rapid publish/unpublish followed by Room disconnect could leave publisher negotiation waiting for an answer on a closed peer connection. The detached task retained the session and native factory threads through its ten-second timeout and retry, violating the application's five-second lifecycle cleanup budget.

Session close now cancels and joins the one owned fast negotiation task and its debouncer before closing signaling and peer connections. Creation and shutdown use the same locks and a closed-session guard. Debouncer cancellation also interrupts an already running future. The public API, sender ownership, ICE eligibility and resource thresholds are unchanged.

Focused validation:

  • Real project SFU, identical production Engine publisher, 50 lifecycle cycles: published SDK .8 failed at +4 handles/+2 threads after 5065 ms cleanup; candidate passed at 0/0, zero pending callbacks and zero cleanup wait.
  • Two exact-source Rust debouncer tests passed: cancellation before execution and cancellation/join during an indefinitely pending future.
  • Built with build.cmd release-tests; no full local SDK or capture matrix was run.
  • The hosted application's original failure (+99 handles/+12 threads) remains documented, as do separate unknown-publication event warnings.

The consolidated Rust patch passes reverse applicability verification. Candidate normalized tree: 8f5c3220de048f23a8f17b4a65653be3c5b11179.

Prerequisite for syrnike13/syrnike13-app#126 and planned SDK .9 release. Source details and evidence are in docs/negotiation-shutdown.md.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Новые возможности

    • Добавлена поддержка предварительно закодированных H.264-видеопотоков.
    • При потере последовательности кадров автоматически запрашивается новый ключевой кадр, предотвращая передачу повреждённого видео.
    • Улучшена замена опубликованных медиатреков с повторным использованием соединений и корректным обновлением параметров.
    • Для предварительно закодированного видео снято ограничение максимальной стартовой скорости передачи.
  • Исправления

    • Устранены утечки ресурсов при завершении сессий.
    • Отложенные операции переговоров и публикации теперь корректно отменяются и завершаются до закрытия соединений.
  • Документация

    • Добавлено описание изменений в завершении переговоров и работы новой версии SDK.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: f88c3489-e31e-4994-821b-1c04a58cdadf

📥 Commits

Reviewing files that changed from the base of the PR and between 83c2bd5 and da6ae12.

📒 Files selected for processing (3)
  • docs/negotiation-shutdown.md
  • patches/client-sdk-rust/0001-native-v2-runtime.patch
  • patches/client-sdk-rust/README.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Добавлена поддержка pre-encoded H.264 кадров через FFI и pass-through encoder. Добавлено переиспользование RTP-трансиверов. Завершение RTC-сессии теперь отменяет и ожидает negotiation и debouncer задачи. Обновлены владение WebRTC-ресурсами, обработчики событий и документация.

Changes

RTC runtime

Layer / File(s) Summary
Завершение negotiation и debouncer
patches/client-sdk-rust/0001-native-v2-runtime.patch, docs/negotiation-shutdown.md, patches/client-sdk-rust/README.md
NegotiationQueue и Debouncer поддерживают отмену с ожиданием завершения. RtcSession::close выполняет drain задач до закрытия signaling и PeerConnection. Документация описывает проверки и результаты тестов.
Поток pre-encoded H.264
patches/client-sdk-rust/0001-native-v2-runtime.patch
FFI получает закодированные H.264 кадры. PassthroughVideoEncoder проверяет непрерывность access units и запрашивает keyframe при разрыве последовательности.
Переиспользование RTP-трансиверов
patches/client-sdk-rust/0001-native-v2-runtime.patch
Удалённые трансиверы помещаются в пул и восстанавливаются для новых треков. SDP обновляет MSID согласно текущему ID трека.
Владение WebRTC-ресурсами и событиями
patches/client-sdk-rust/0001-native-v2-runtime.patch
PeerConnectionFactory создаёт сетевые объекты и передаёт PortAllocator. RTC- и E2EE-обработчики используют слабые ссылки. Неактивный RtpSender возвращает nullptr для трека.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: ⚪ Minimal · up to da6ae

RTC session shutdown now cancels and joins negotiation and debouncer work before connection teardown, preventing retained session resources without an identified current-head merge risk.

Sequence Diagram(s)

sequenceDiagram
  participant FfiClient
  participant VideoSource
  participant PassthroughVideoEncoder
  participant WebRTC
  FfiClient->>VideoSource: CaptureEncodedVideoFrame
  VideoSource->>PassthroughVideoEncoder: передать H.264 access unit
  PassthroughVideoEncoder->>WebRTC: передать кадр или запросить keyframe
Loading

Suggested reviewers: xianshijing-lk

Poem

Кролик несёт H.264 кадр,
Проверяет цепочку в такт.
Трансивер снова в строй идёт,
Debouncer тихо task сдаёт.
Сессия закрылась — всё чисто.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Заголовок точно описывает основное изменение: завершение задачи переговоров издателя при остановке RTC-сессии.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/126-negotiation-shutdown

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@tiredIsa
tiredIsa merged commit 049ec1b into native-v2 Sep 6, 2026
25 checks passed
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.

1 participant