Skip to content

fix: WebRTC data-channel requests keep running after disconnect and can hang peer cleanup#834

Closed
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-273f3989
Closed

fix: WebRTC data-channel requests keep running after disconnect and can hang peer cleanup#834
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-273f3989

Conversation

@sam-saffron-jarvis

Copy link
Copy Markdown
Contributor

What changed

  • created a data-channel-scoped request context in runDataChannel and cancel it before waiting for in-flight handlers during disconnect, idle-timeout, and reader-exit teardown paths
  • made data-channel send failures stop the channel so all in-flight WebRTC requests get canceled promptly
  • updated dispatchRequest to create a per-request child context and taught dcResponseWriter to fail closed by canceling that request context on the first send error
  • added a regression test proving a handler blocked on r.Context().Done() is released when response-frame delivery fails

Why this is high-value

A closed browser tab or broken data channel could previously leave long-lived WebRTC handlers running under the peer's long-lived context, while peer teardown blocked in wg.Wait(). That can strand streaming /v1/responses work, model runs, and goroutines until the handler exits on its own. This fix makes disconnects and transport write failures actively cancel request contexts, so cleanup and reconnect behavior are reliable instead of waiting on orphaned handlers.

Validation

  • gofmt -w internal/webrtc/peer.go internal/webrtc/peer_test.go
  • go build ./...
  • go test ./...
  • git diff --check

@SamSaffron

Copy link
Copy Markdown
Owner

Closing as requested: the high-confidence fixes have been applied or superseded in the current working tree.

@SamSaffron SamSaffron closed this Jun 18, 2026
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.

2 participants