Skip to content

Improve thread safety of Connection.Disconnect(), avoid InvalidOperationException if Disconnect is called twice #937

Description

@jpdillingham

The following is unsynchronized, and it contains a time-of-check time-of-use bug with ConnectionState. There are 3-4 different independent callers of Disconnect() and this could cause a number of unexpected results. Add a lock or semaphore to make sure the whole method is synchronized.

        public void Disconnect(string message = null, Exception exception = null)
        {
            if (State != ConnectionState.Disconnected && State != ConnectionState.Disconnecting)
            {
                message ??= exception?.Message;

                ChangeState(ConnectionState.Disconnecting, message);

                InactivityTimer?.Stop();
                WatchdogTimer.Stop();
                Stream?.Close();
                TcpClient?.Close();

                ChangeState(ConnectionState.Disconnected, message, exception);
            }
        }

The associated ChangeState method is well synchronized, but fails to account for the fact that DisconnectTaskCompletionSource will throw if SetException or SetResult are called after the completion source has been completed already. Change these to TrySetException and TrySetResult

        protected void ChangeState(ConnectionState state, string message, Exception exception = null)
        {
            ...
            else if (State == ConnectionState.Disconnected)
            {
                ...
                if (exception != null)
                {
                    DisconnectTaskCompletionSource.SetException(exception);
                }
                else
                {
                    DisconnectTaskCompletionSource.SetResult(message);
                }
            }
        }

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions