doghappy / socket.io-client-csharp

socket.io-client implemention for .NET
MIT License
729 stars 125 forks source link

possibly multiple OnConnected events firing unexpectedly #246

Open AlexSapple opened 2 years ago

AlexSapple commented 2 years ago

Hi,

since upgrading to 3.0.4 (from 2.3.0) running on .Net6 - I've started seeing some strange behaviour in relation to the OnConnected event appearing to fire twice (where it fired only once before).

I've got the following code that handles socket connection and uses an AutoResetEvent to hold the thread until the onConnected fires (so bool success = autoResetEvent.WaitOne(10000); implies that either 10 seconds elapses or the event fires and the signal is given via autoResetEvent.Set();. In the previous version, this worked without issue, but now I'm getting an exception that states that the AutoResetEvent is already closed when attempting to call Set() this could only happen if 10 seconds elapsed (which is hasn't) or and exception is thrown within the try/catch block (which in this case, it has not)

so that leads me to the belief that the onConnected event has fired multiple times (the first time resulted in the thread continuing and closing the AutoResetEvent, and the second resulting in exception as AutoResetEvent is already closed.

is this expected behavior?

    protected async Task<bool> ConnectSocketToMediaAsync(Uri endpointAddress, string bearerToken = null)
    {
        SocketIOOptions options = new SocketIOOptions
        {
            EIO = 4
        };

        //if we have been given a bearer token, then ensure it's added.
        if (!string.IsNullOrWhiteSpace(bearerToken))
        {
            options.Query = new Dictionary<string, string>
            {
                {"token", bearerToken}
            };
        }

        Socket ??= new SocketIO(endpointAddress, options);

        AutoResetEvent autoResetEvent = new AutoResetEvent(false);
        Socket.OnConnected += (_, _) => autoResetEvent.Set();

        try
        {
            await Socket.ConnectAsync();
            Socket.OnDisconnected += UnexpectedDisconnect;
        }
        catch
        {
            autoResetEvent.Close();
            return false;
        }

        bool success = autoResetEvent.WaitOne(10000);
        autoResetEvent.Close();
        return success;
    }
doghappy commented 2 years ago

It is not expected. Can you show exception information or logs?

By the way, in most cases, you configure EIO incorrectly, please set it to 3 and try it

AlexSapple commented 2 years ago

thanks @doghappy

So I completely removed the EIO as newest documentation indicates it's not needed - but I believe 4 was correct as trying 3 resulted in the socket not successfully connecting.

in terms of the behaviour - the socket does connect and I do get acks in subsequent calls - for example in the following code - the joinRoomAsync() method emits and gets a successful ack and the GetMeetingRoomAsync() method emits and gets a response listing other connected peers - this is all working well.

  if (await ConnectSocketToMediaAsync(Configuration.MediaServerUri, _bearerToken)
                && await JoinRoomAsync(roomId, Configuration.PeerId, Configuration.PeerId,
                    Configuration.PeerId, Configuration.PeerId, Configuration.PeerId))
            {
                return await GetMeetingRoomAsync();
            }

so it does appear to just be spurious errors coming from the OnConnected of the ConnectSocketToMediaAsync() event.

in terms of logs, well I've not really got any - the error is just the one I mention from the AutoResetEventcalling Set() multiple times.

I'd like to extract the SocketIO messages but not sure what tool to use for server to server type connections - maybe something like wireshark? (I use fiddler for tracing front end connections).

to try and resolve in the short term, I'll deregister my event listener immediately after the initial connection.