JKorf / Bittrex.Net

A C# .Net wrapper for the Bittrex web API including all features easily accessible and usable
MIT License
141 stars 62 forks source link

Synchronous wait in WebsocketCustomTransport.OnStart cause threadpool starvation #205

Closed EricGarnier closed 2 years ago

EricGarnier commented 2 years ago

@JKorf , I notice that the OnStart method in WebsocketCustomTransport make a synchronous call line 70

        if (!websocket.ConnectAsync().Result)

Unfortunately, this method is used by SignalR in an async call. This result in a blocked thread in the thread pool, and other problems.

Maybe a good solution could be to keep the task created by ConnectAsync in a field, and on completion and error, call StartFail.

But I am not sure.

HTH Eric

JKorf commented 2 years ago

Hi Eric, I'm not really sure how the internals of SignalR work, maybe you have some more insight in this. Shouldn't the OnStart wait for the connection to be established (or failed)? Doesn't the SignalR internal state machine assume the connection is made when the OnStart returns without an error? Again I'm not sure about any of this :)

EricGarnier commented 2 years ago

Maybe this can give us some insight WebSocketTransport

        protected override void OnStart(IConnection connection, string connectionData, CancellationToken disconnectToken)
        {
            _disconnectToken = disconnectToken;
            _connection = connection;
            _connectionData = connectionData;

            // We don't need to await this task
            ConnectAndHandleConnection().ContinueWith(task =>
            {
                if (task.IsFaulted)
                {
                    TryFailStart(task.Exception);
                }
                else if (task.IsCanceled)
                {
                    TryFailStart(null);
                }
            },
            TaskContinuationOptions.NotOnRanToCompletion);
        }

It looks like there is no need to wait for the connection.

HTH Eric

JKorf commented 2 years ago

Hi Eric, I've adjusted it so it doesn't wait for the connection anymore, hope that helps!

EricGarnier commented 2 years ago

Great. As the point occurs only randomly, I cannot be sure its corrected. So I close the issue, and will re-open should it occurs again.

Thanks. Eric