Fix disconnect() while reconnecting waiting for the pong timeout

[Keith-wright]

What does this PR do?

Fixes #376. Calling disconnect() while the connection was RECONNECTING left it stuck in DISCONNECTING until the ping and pong timers ran out, which takes up to activityTimeout plus pongTimeout.

In the RECONNECTING state the previous socket has already closed, so calling close() on it never produces an onClose callback. disconnect() now moves straight to DISCONNECTED in that state and cancels the pending reconnect timer, so the timer thread shuts down as well.

The reconnect task now checks the connection state on the event thread, so it can't run at the same time as disconnect() and open a new socket after the connection was closed. This race existed before this PR.

disconnect() during CONNECTING isn't changed. Closing a socket in the middle of its handshake still produces onClose, although it can be delayed if the TCP connect is blocked.

It also adds tests for these cases.

This PR will carry the 2.4.5 release. The release automation builds the changelog from this PR's description, so the CHANGELOG section lists everything in the release, including changes from other PRs.

Closes #374.

CHANGELOG

• [FIXED] Calling disconnect() while reconnecting now moves to DISCONNECTED immediately instead of waiting for the pong timeout (disconnect() while RECONNECTING leaves connection stuck in DISCONNECTING until pong timeout (2.4.4) #376).
• [FIXED] A ChannelAuthorizer that returns null or an empty string now causes an AuthorizationFailureException instead of a crash (NullPointerException when authorizing private channel subscription #372).
• [FIXED] Repeated reconnect attempts after user sign-in no longer crash with an IllegalStateException (IllegalStateException error is thrown when the server is closed on a logged-in server #367).
• [FIXED] The PusherEvent constructor now stores the user ID under user_id, so getUserId() returns it (Fix PusherEvent user id wrong key. Change userId to user_id #350).
• [ADDED] PusherOptions.setPath for connecting to a custom URL path (added the ability to specify a path #371).
• [ADDED] setSslSocketFactory on HttpChannelAuthorizer and HttpUserAuthenticator, for custom TLS setups such as mutual TLS (Allow custom SSL socket factory for BaseHttpAuthClient #359).

[dchankhour-godaddy]

LGTM, I'd approve, but GitHub only allows approvals from accounts with repository access.

Thanks for picking up #376. I reviewed this against the full WebSocketConnection and Factory sources, and it fixes the case we reported. With disconnect() called during RECONNECTING, the connection now reaches DISCONNECTED immediately and its threads shut down, instead of waiting out the ping/pong timeouts. Cancelling reconnectTimer matters here because shutdownThreads() uses timers.shutdown(), and ScheduledThreadPoolExecutor still runs delayed tasks that were already scheduled after shutdown by default.

A few follow-ups. I've left inline comments on the first two:

1. Reconnect timer race (existed before this PR). The reconnect runnable runs on the timers thread outside eventLock, so its state == RECONNECTING check isn't serialized with disconnect(). Details are inline.
2. The PR description doesn't match the diff. The CHANGELOG section lists six entries (#372, #367, #350, #371, #359), but the diff only contains the #376 fix and doesn't touch CHANGELOG.md. Also, Closes #374 would auto-close "Maintenance status for this repo" when this merges, and that issue looks unrelated. Could you trim the description so only the relevant issue gets closed?
3. CONNECTING isn't covered. That seems reasonable: close() on a socket that's mid-handshake should still produce onClose, although it can be delayed if the TCP connect is blocked. It might be worth mentioning in the PR.
4. Test suggestion. Capture the scheduled reconnect runnable, run it after disconnect(), and assert that newWebSocketClientWrapper isn't called a second time. That covers the cancel/race path end-to-end.

It would be great to see this in a 2.4.5 release.

[Keith-wright]

Thanks for the thorough review, @dchankhour-godaddy .

I moved the reconnect check onto the event thread as you suggested, and disconnect() now removes the old socket's listener too. Both changes are in the latest commit.

I've added a note about the CONNECTING case to the description.

The CHANGELOG section is intentional btw. This PR will carry the 2.4.5 release, and our release automation builds the changelog from this PR's description, so it lists changes from the other PRs in the release. I've added a note to the description to explain this. "Closes #374" stays as well, as that issue asks whether the library is still maintained, and this work and the subsequent 2.4.5 release resolves that issue.

Lastly, I added a test that runs the captured reconnect task after disconnect() and checks that no second socket is created.

[dchankhour-godaddy]

Thanks @Keith-wright, I re-reviewed 21344a6 and this looks good to me. 👍

• Doing the reconnect check on the event thread, under eventLock, resolves the race with disconnect(). A task that fires late now sees DISCONNECTED and does nothing.
• removeWebSocketListener() in the new disconnect() branch, and the new reconnect-after-disconnect test, both look good.
• The explanation for the CHANGELOG and Maintenance status for this repo #374 makes sense, thanks for clarifying.

One tiny non-blocking nit: if the reconnect task fires after shutdownThreads(), queueOnEventThread lazily recreates the event executor. The queued work does nothing, but it leaves one idle daemon thread behind. It's rare and harmless, so this is just a note in case you want to guard it.

The CI build jobs still appear to be pending, probably waiting on workflow approval. Looking forward to 2.4.5!