Fix race condition with the reader task in ObservableSocket.cs #15
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.
Hi,
using NEventSocket under Ubuntu Linux .NET 6 on the same machine together with FreeSWITCH, we encountered unexpected timeouts in relatively low load situations that looked like the following:
Then we run another client we develop in a loop alongside while the load test was running, which was happily authenticating with over 300 connections / seconds to the mod_event_socket endpoint (port 8021, sub millisecond authentication times to FreeSWITCH), which let us strongly believe that FreeSWITCH and neither the load wasn't the problem here.
Then looking closer at the NEventSocket source code we found that - if the task that reads the
TcpClientstream - receives packets faster than the first subscriber is actually subscribed at the RxSubject, theauth/requestpacket may get lost.The actual problem here is that the
Defer()operator in Rx is invoked before the subscriber is connected to the subject. So if theTcpClientconnects, and FreeSWITCH responds immediately, and the Task starts running before thesubject.Subscribe()has returned,subject.HasSubscribersisfalse, and the invocation ofsubject.OnNext()dismisses the packet, causing a subsequent timeout for the firstauth/request.This PR fixes that from the perspective of someone that has no idea how the idiomatic solution in Rx would look like. My solution is to intercept the
Subscribeinvocation and start the reader task after that (once only that is, of course). That worked for us so far.@iamkinetic @ajgolledge