andreykaipov / goobs

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

bugfix(client): panic: send on closed channel #179

Closed xaionaro closed 3 weeks ago

xaionaro commented 1 month ago

Fix:

panic: send on closed channel

goroutine 751872 [running]:
github.com/andreykaipov/goobs.(*Client).writeEvent(...)
    /home/xaionaro/.gvm/pkgsets/go1.22.1/global/pkg/mod/github.com/andreykaipov/goobs@v1.4.1/client.go:363
github.com/andreykaipov/goobs.(*Client).handleOpcodes(0xc0020c81a0, 0xc0013846c0)
    /home/xaionaro/.gvm/pkgsets/go1.22.1/global/pkg/mod/github.com/andreykaipov/goobs@v1.4.1/client.go:338 +0x5a5
created by github.com/andreykaipov/goobs.(*Client).connect in goroutine 751658
    /home/xaionaro/.gvm/pkgsets/go1.22.1/global/pkg/mod/github.com/andreykaipov/goobs@v1.4.1/client.go:200

Essentially by design we should close a channel only after we finished all possible writing to it. Thus moving the close statement of channel IncomingResponses to be called right after the writer to the channel is finished.

andreykaipov commented 1 month ago

Hi @xaionaro - thanks for the PRs! Sorry it took a while. I didn't have time to figure out why the CI was failing in this repo. But now that's fixed in https://github.com/andreykaipov/goobs/pull/181, can you rebase your PRs or pull in the latest changes from main to retrigger the CI? I wanna make sure that passes before merging them in! 🙏

Alternatively I think there's a checkbox somewhere that allows you to allow edits from maintainers and I can rebase for you.

xaionaro commented 1 month ago

Wait, let me re-figure-out why I wrote the patch the way I did it (to make sure I haven't missed anything). Will re-open after the recheck.

xaionaro commented 1 month ago

@andreykaipov It seems like my fix was correct (added a comment into the code), but a bit incomplete. Added a one more commit to fix some other potential (but less likely) race conditions.

I've also rebased my project on top of these commits, so to be extra safe we can wait a week and see if it works fine.