Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix session ID deadlockwhen opening a VoiceConnection #1408

Merged
merged 4 commits into from
Aug 13, 2023

Conversation

denverquane
Copy link
Contributor

@denverquane denverquane commented Jul 17, 2023

Closes #829

When checking the Voice Session ID in VoiceConnection.open(), the sessionID is populated asynchronously when websocket messages come in, see wsapi.go.

However, open() still holds the lock on the session, so the ID can never be populated, and we loop and accomplish nothing until releasing the lock upon exiting the function.

This PR addresses this deadlock bug by momentarily releasing the lock anytime the process sleeps, allowing other processes to populate the sessionID appropriately - while not sacrificing concurrency safety by releasing the lock entirely.

@FedorLap2006 FedorLap2006 added fix Pull requests and issues related to bug fixes and structural inconsistencies voice Voice related issues and pull requests labels Aug 12, 2023
Fix formatting of the documentation comment for VoiceConnection.Speaking function
voice.go Outdated Show resolved Hide resolved
@FedorLap2006 FedorLap2006 changed the title Fix: "fail to receive voice session ID in time" deadlock Fix session ID deadlock in VoiceConnection.Open Aug 13, 2023
@FedorLap2006 FedorLap2006 changed the title Fix session ID deadlock in VoiceConnection.Open Fix session ID deadlockwhen opening a VoiceConnection Aug 13, 2023
@FedorLap2006 FedorLap2006 merged commit d8eb5a5 into bwmarrin:master Aug 13, 2023
8 checks passed
@FedorLap2006
Copy link
Collaborator

Thanks for the contribution!

@FedorLap2006
Copy link
Collaborator

I did the edits myself, to not hold it off for longer. Hope you understand and sorry for the wait.

FedorLap2006 added a commit that referenced this pull request Aug 13, 2023
* Unlock when checking voice connection sessionID to prevent deadlock

* Move lock to preserve concurrency safety, while allowing the sessionID to be populated

* style: formatting

Fix formatting of the documentation comment for VoiceConnection.Speaking function

* feat: reword explanatory comment

---------

Co-authored-by: Fedor Lapshin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull requests and issues related to bug fixes and structural inconsistencies voice Voice related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Voice websocket closed unexpectedly at random times
2 participants