anycable / anycable-go

AnyCable real-time server
https://anycable.io
MIT License
375 stars 65 forks source link

Fix defer Redis Connection Close before checking sentinel role #144

Closed DmitryBochkarev closed 2 years ago

DmitryBochkarev commented 2 years ago

What is the purpose of this pull request?

As a result of reviewing the code, I found a place with potential leaked connections to Redis, in a situation where the role is not correctly set for sentinel. As you can see connections will be lost as a result of reconnects until the GC comes back on or the application is quit.

I have not reproduced this situation as I am currently limited in time. But I think that it is possible to draw such a situation in mind and to draw the conclusions that I came to.

What changes did you make? (overview)

As a fix, I moved the delayed closing of the connection before checking the sentinel role, this ensures that after exit the connection will be closed.

Is there anything you'd like reviewers to focus on?

Checklist

palkan commented 2 years ago

Thanks for catching this!