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 multi-goroutine request error #54

Merged
merged 4 commits into from
Sep 15, 2023

Conversation

ImmortalD
Copy link
Contributor

fix multi-goroutine bug and add multi-goroutine request test

1. panic: concurrent write to websocket connection
2. request GetSceneList: mismatched ID: expected response with ID c30f63f4-0065-4cb0-5873-19e12a676ee1, but got 97362273-a871-415b-6610-45894239012e

ImmortalD and others added 4 commits September 11, 2023 16:42
1. panic: concurrent write to websocket connection
2. request GetSceneList: mismatched ID: expected response with ID c30f63f4-0065-4cb0-5873-19e12a676ee1, but got 97362273-a871-415b-6610-45894239012e
@andreykaipov
Copy link
Owner

Thank you! What a simple solution! I was able to reproduce it and added a sync.WaitGroup{} to the test to make it more reliable.

@andreykaipov andreykaipov merged commit ec56dc7 into andreykaipov:master Sep 15, 2023
@KopiasCsaba
Copy link

I sadly don't have time to repeat it now, but I still got that error message with the latest one when hitting OBS hard.
I have added a mutex wrapper around it myself (before discovering this) also and that solved it, but that might suggest that something is still not ok there...

@ImmortalD
Copy link
Contributor Author

I sadly don't have time to repeat it now, but I still got that error message with the latest one when hitting OBS hard. I have added a mutex wrapper around it myself (before discovering this) also and that solved it, but that might suggest that something is still not ok there...

The solution isn't perfect; in practice, it effectively turns the low-level operations into a single-threaded process for sending OBS messages (excluding authentication and closing). Implementing the ability for multiple goroutines to send messages and using pure channels may be challenging, but speed might not be as critical. Regarding the persistence of the error message, it's still possible because we've only added locking when sending messages (OBS messages). There's no locking during authentication (creating the OBS client), and there's no locking during the client's closure. Therefore, you should ensure that no goroutines are sending OBS messages during authentication, and similarly, you should ensure that no goroutines are still sending messages when the OBS client is closing. Otherwise, you might still encounter that error message.

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.

3 participants