andreykaipov / goobs

Go client library for OBS Studio
Apache License 2.0
137 stars 22 forks source link

fix multi-goroutine request error #54

Closed ImmortalD closed 1 year ago

ImmortalD commented 1 year ago

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
andreykaipov commented 1 year ago

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.

KopiasCsaba commented 1 year ago

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 commented 1 year ago

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.