Repository navigation
Conversation
Reading the previous connection and registering the new one were two steps, so two connections of a station racing on two IO threads could read the same previous connection. One of them was then overwritten without being closed: it stayed open, kept receiving the station's messages, and its close was never notified. Two first connections racing both notified onWsConnectHandler. Read and swap the connection under a lock of the station, so that concurrent connections each replace, and close, a distinct connection. The swap still runs under the lock of the connection it replaces, so a control frame of that connection being notified completes before the replacement. The lock of a station is dropped once no thread holds or waits on it: station ids come from the request path, so locks kept forever would grow with every id a client sends. Closes IZIVIA#123
The connect, reconnect and close events of a station were notified from the IO thread of each connection, outside any lock. A station closing a connection and opening the next one right away could have its close notified after the connect of the next connection, leaving the listener with the station disconnected while it was connected. Concurrent connections could also notify a reconnection before the connection it follows. Register or remove a connection and notify the matching event as one step, under the lock of the station, on connection, close and shutdown. Listeners now run under that lock: a slow listener delays the connection and close of its station. Replaced connections are closed once the lock is released, since their close notifies nothing, and also when a listener fails. Closes IZIVIA#142
… fails A connect or reconnect listener that threw left the new connection registered with none of its socket handlers attached: its close was never handled nor notified, its messages were never read so a call to the station waited for its timeout, and its next connection was taken for a reconnection of a connection nobody could use. The connection listeners are now observers: what they throw is logged and the connection is set up as usual, still under the station lock, so the registry matches what the listeners were told and the next connection of the station is notified as a reconnection. The handlers of the station are resolved before the connection is registered, and so before its connection listener runs, so that their failure leaves nothing registered. Closes IZIVIA#149
|
18 tasks
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.
Stacked on #150 (#123, #142): its two commits, which serialize the lifecycle of each station under a per-station lock (
StationLocks), show here until it merges; only the last commit is new. A connect or reconnect listener that threw left the new connection registered without any socket handler, so its close was never notified, calls to the station timed out and its next connection was taken for a reconnection; these listeners are now observers whose failures are logged, still under the station lock, so the station stays reachable and the next connection is notified as a reconnection. The handlers of the station are resolved before the connection is registered, so that their failure leaves nothing registered, and theEventsListenersKDoc and the README state the new behaviour. In-memory socket tests cover a throwing connect and reconnect listener (call answered, close notified, next event) and throwing handlers, each checked to fail when its fix is removed. Refusing a station by throwing from a listener never worked; an explicit admission from the upgrade request headers is proposed in #152.Closes #149