Fix for #425 (Session expected to be closed in OnInvalidateSessionAsync, while it is always open)
Created by: uwx
Summary
Fixes #425 (closed) (misleadingly titled "Bot Stays offline in Discord after it Reconnects due to Discord Cloudfare reboot")
Details
The cited issue is a result of Emzi's ported deadlock fix code (ironically).
When Discord sends an Opcode 9 Invalid Session payload, SessionLock will (likely always) be in nonsignaled (blocking) state (as is set in the HELLO handler), but the OP9 handler expects it to be open, returning otherwise. The only situation where that behavior would be useful would be if the WebSocket payload was received twice in succession, which isn't all that great considering the design of the session lock means it'll discard the payload every time...
Anyway, if you discard this seemingly impossible edge case, the most reasonable behavior would be to never return early from OnInvalidateSessionAsync. Instead, my implementation ensures there is always a session running (so that further user-induced session starts will be discarded, as they should), and maintains the previous behavior of locking the socket (as we're effectively starting a new session).
As you may have gathered, and as others have told, this bug has nothing to do with Discord's CloudFlare proxy rebooting. It will happen in any situation where a reconnect results in an OP9 instead of READY/RESUMED.
Changes proposed
- Consume the SessionLock in OnInvalidateSessionAsync without returning and deadlocking the client
Notes
Thanks dearly to @WamWooWamWooWamWooWamWooWamWooWam for finding out that it was the locking code that was causing the deadlock and not any of the incredibly absurd things I had thought.
I haven't tested this completely yet, either. I will do that in a moment.
Other possible solutions
- A proper system to handle locking for both session connect and session establish so that duplicate OP9s get handled. Because I can't imagine such a thing can happen, I think this would be wholly unnecessary and confusing.