Fixed duplicates reconnect mentionned in issue #89 #135
Conversation
There was a problem hiding this comment.
Thanks for working on this! I left you a few comments, but I'd like to talk about the general strategy for solving this. If you look at the issue you referenced, #89, it talks about tigertext@e39460e#diff-bd6ddbbcfe0037b5a08d70a3b5f4930c Did you look at that strategy?
Also, we'd need tests to make sure we don't regress on this issue.
I looked at what they did but for some reason it did not work for me, I'm using a C# server and even with their fix I still managed to get duplicate reconnections attempts when killing the server or unplugging machine running the server. I won't pretend to fully understand what's happening in this lib but I'm pretty confident my fix works (well at least for my use case). And I guess that's why you mentioned adding tests to prevent regression, however I have no Idea on how to do that in practice yet. |
|
The reason I suspect that other commit would be a good fix is that it's effectively doing the same thing as your timeout fix, but still leaves us open to the possibility that there's a condition where something is re-marking the connection as open when it shouldn't. If that's true, that's the root cause of the problem and we should fix that! btw, if you've got an issue with |
|
@Hedgestock @joeybaker I think this PR #125 has already fixed the issue mentioned here, but this PR has reverted that fix, maybe by mistake when solving conflictes, please check. As for tigertext@e39460e#diff-bd6ddbbcfe0037b5a08d70a3b5f4930c , it dose not really solve the duplicate reconnection issue, check my comment HERE |
In some cases the server going down can lead to multiple triggers of the
onConnectionClosed()method, resulting in multiple reconnection timeouts being created. I fixed this problem by only allowing one timeout to be running at a time.