Remove lock from sessionWS.SendBytes #1140
Merged
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.
There are six different calls to s.Lock() in sessionWS. Three of these locks are around calls to SetWriteDeadline and WriteMessage on the WebSocket. WriteMessage will block for up to the timeout set in SetWriteDeadline, which has a default of 5000 milliseconds. This lock is also acquired by SendBytes to determine if the session has been stopped. This is a problem as the event processing methods in StatusRegistry and Track call SendBytes directly on the Session. If one of the Sessions being written to has delays in writing data, this could cause backups in the event processing.
The simplest approach (this commit) is to just remove the lock within SendBytes. To avoid a race condition with writing to a closed channel, we need to not close the outgoingCh channel in the Close method. This is safe to do as channels do not need to be explicitly closed and will automatically be cleaned up by the garbage collector. It should also be possible to remove the lock entirely from sessionWS, but that is a much larger change.