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
|
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.
OcppWampServerAppnow registers or removes a station's connection and notifies the matching connect, reconnect or close event as one step, under a per-station lock, so concurrent connections each replace and close a distinct connection (#123) and a close is never notified after the connection that follows it (#142). The locks live inStationLocks, which drops a station's lock once no thread holds or waits on it, since station ids come from the request path and must not grow memory without bound. A replaced connection is closed outside the lock, even when a listener fails, and theEventsListenersKDoc and the README now state the ordering guarantee and that lifecycle listeners must not block. In-memory socket tests cover each ordering (close vs. next connection, reconnection vs. its connection, shutdown, case-insensitive ids) plus a race stress test, and each one was checked to fail when its lock is removed. This supersedes #143; the pre-existing case of a throwing connect listener is tracked in #149.Closes #123
Closes #142