Optionally disconnect SWT clients that aren't receiving messages#4099
Open
liamcary wants to merge 1 commit intoMirrorNetworking:masterfrom
Open
Optionally disconnect SWT clients that aren't receiving messages#4099liamcary wants to merge 1 commit intoMirrorNetworking:masterfrom
liamcary wants to merge 1 commit intoMirrorNetworking:masterfrom
Conversation
…t processing incoming messages
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an optional max count to the SWT connection sendQueue on the server side. If the sendQueue exceeds this count, the server will forcefully disconnect the connection.
In my testing it is fairly common for a client's websocket to stop receiving incoming messages, whether due to a bad connection, or CPU lag or browser/client errors. If the connection remains open but doesn't send message acknowledgements, the server's SendLoop.Loop will block on stream.Write and the sendQueue can accumulate thousands of ArrayBuffer instances.
This isn't a memory leak as such, since the ArrayBuffers in the sendQueue will be returned to the buffer pool if the client disconnects or the connection recovers. But the memory usage of the server application will be permanantly inflated until the server restarts.
The amount of excess memory without this feature could theoretically be endless, until the application crashes. In many cases the Disconnect Inactive Connections feature would kill stalled clients, but that feature is disabled by default. A very slow/stalled client could also be considered active if its still processing messages, but slower than they are enqueued.
I considered adding a sensible default value, or suggesting a sensible value in the tooltip, but it varies greatly depending on the project. SendRate and batchSend especially will greatly affect the number of messages per second. I currently use this in production on two games with the max count set to 512, using a send rate of 30 and batch send enabled.
This PR conflicts with my other open PR: #4098
If both PR's are merged in, WebsockerServer.Send will need to be updated to return false after disposing a connection due to reaching the max send queue count.