8000 fix(voice): Conn.Close blocking forever by shi-gg · Pull Request #547 · disgoorg/disgo · GitHub
[go: up one dir, main page]

Skip to content

fix(voice): Conn.Close blocking forever - #547

Merged
topi314 merged 4 commits into
disgoorg:masterfrom
shi-gg:fix/voice-close-blocking
Jun 5, 2026
Merged

topi314 merged 4 commits into
disgoorg:masterfrom
shi-gg:fix/voice-close-blocking

Conversation

@shi-gg
@shi-gg shi-gg commented Jun 2, 2026
Copy link
Copy Markdown
Contributor

connImpl.Close() deferred gateway.Close() and udp.Close() to after a select that waited on c.closedChan. The channel was only filled by HandleVoiceStateUpdate when Discord confirmed the voice leave.

In linkdave's case, voiceStateUpdateFunc is a no-op — so Discord never received the leave and closedChan was never signaled. Close() blocked until context timeout, with all resources still open during the wait.

I verified this behavior with a modified version of linkdave using oldVC.Close(context.Background()).

2026-06-02.15-43-44-compressed.mp4

Copilot AI review requested due to automatic review settings June 2, 2026 13:59
Copilot AI left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adjusts voice connection shutdown signaling and teardown to avoid blocking on closedChan and to close transports immediately when Close() is invoked.

Changes:

  • Make the closedChan notification in HandleVoiceStateUpdate non-blocking via select { case ... default: }.
  • Simplify Close() by removing the wait on closedChan and directly closing the gateway/UDP connection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread voice/conn.go Resolved
Comment thread voice/conn.go Resolved
@topi314
topi314 merged commit 18dc4c2 into disgoorg:master Jun 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants

0