Merge pull request #1775 from aspnet/release/2.1

Fix internal HubConnection state when handshake fails (#1774)
This commit is contained in:
BrennanConroy 2018-03-29 16:35:55 -07:00 committed by GitHub
commit 5cfc03ffe9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 36 additions and 15 deletions

View File

@ -132,24 +132,26 @@ namespace Microsoft.AspNetCore.SignalR.Client
// Start the connection
var connection = _connectionFactory();
await connection.StartAsync(_protocol.TransferFormat);
_connectionState = new ConnectionState(connection, this);
var startingConnectionState = new ConnectionState(connection, this);
// From here on, if an error occurs we need to shut down the connection because
// we still own it.
try
{
Log.HubProtocol(_logger, _protocol.Name, _protocol.Version);
await HandshakeAsync();
await HandshakeAsync(startingConnectionState);
}
catch (Exception ex)
{
Log.ErrorStartingConnection(_logger, ex);
// Can't have any invocations to cancel, we're in the lock.
await _connectionState.Connection.DisposeAsync();
await startingConnectionState.Connection.DisposeAsync();
throw;
}
// Set this at the end to avoid setting internal state until the connection is real
_connectionState = startingConnectionState;
_connectionState.ReceiveTask = ReceiveLoop(_connectionState);
Log.Started(_logger);
}
@ -494,15 +496,15 @@ namespace Microsoft.AspNetCore.SignalR.Client
}
}
private async Task HandshakeAsync()
private async Task HandshakeAsync(ConnectionState startingConnectionState)
{
// Send the Handshake request
Log.SendingHubHandshake(_logger);
var handshakeRequest = new HandshakeRequestMessage(_protocol.Name, _protocol.Version);
HandshakeProtocol.WriteRequestMessage(handshakeRequest, _connectionState.Connection.Transport.Output);
HandshakeProtocol.WriteRequestMessage(handshakeRequest, startingConnectionState.Connection.Transport.Output);
var sendHandshakeResult = await _connectionState.Connection.Transport.Output.FlushAsync(CancellationToken.None);
var sendHandshakeResult = await startingConnectionState.Connection.Transport.Output.FlushAsync(CancellationToken.None);
if (sendHandshakeResult.IsCompleted)
{
@ -514,7 +516,7 @@ namespace Microsoft.AspNetCore.SignalR.Client
{
while (true)
{
var result = await _connectionState.Connection.Transport.Input.ReadAsync();
var result = await startingConnectionState.Connection.Transport.Input.ReadAsync();
var buffer = result.Buffer;
var consumed = buffer.Start;
var examined = buffer.End;
@ -551,7 +553,7 @@ namespace Microsoft.AspNetCore.SignalR.Client
}
finally
{
_connectionState.Connection.Transport.Input.AdvanceTo(consumed, examined);
startingConnectionState.Connection.Transport.Input.AdvanceTo(consumed, examined);
}
}
}

View File

@ -115,6 +115,25 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
});
}
[Fact]
public async Task StartAsyncWithFailedHandshakeCanBeStopped()
{
var testConnection = new TestConnection(autoHandshake: false);
await AsyncUsing(new HubConnection(() => testConnection, new JsonHubProtocol()), async connection =>
{
testConnection.Transport.Input.Complete();
try
{
await connection.StartAsync();
}
catch
{ }
await connection.StopAsync();
Assert.True(testConnection.Started.IsCompleted);
});
}
[Theory]
[MemberData(nameof(MethodsNamesThatRequireActiveConnection))]
public async Task MethodsThatRequireStartedConnectionFailIfConnectionNotYetStarted(string name)

View File

@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
[Fact]
public async Task ClientSendsHandshakeMessageWhenStartingConnection()
{
var connection = new TestConnection(autoNegotiate: false);
var connection = new TestConnection(autoHandshake: false);
var hubConnection = CreateHubConnection(connection);
try
{
@ -407,7 +407,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
[Fact]
public async Task PartialHandshakeResponseWorks()
{
var connection = new TestConnection(synchronousCallbacks: true, autoNegotiate: false);
var connection = new TestConnection(synchronousCallbacks: true, autoHandshake: false);
var hubConnection = CreateHubConnection(connection);
try
{
@ -436,7 +436,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
public async Task HandshakeAndInvocationInSameBufferWorks()
{
var payload = "{}\u001e{\"type\":1, \"target\": \"Echo\", \"arguments\":[\"hello\"]}\u001e";
var connection = new TestConnection(synchronousCallbacks: true, autoNegotiate: false);
var connection = new TestConnection(synchronousCallbacks: true, autoHandshake: false);
var hubConnection = CreateHubConnection(connection);
try
{

View File

@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
{
internal class TestConnection : IConnection
{
private readonly bool _autoNegotiate;
private readonly bool _autoHandshake;
private readonly TaskCompletionSource<object> _started = new TaskCompletionSource<object>();
private readonly TaskCompletionSource<object> _disposed = new TaskCompletionSource<object>();
@ -38,9 +38,9 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
public IFeatureCollection Features { get; } = new FeatureCollection();
public int DisposeCount => _disposeCount;
public TestConnection(Func<Task> onStart = null, Func<Task> onDispose = null, bool autoNegotiate = true, bool synchronousCallbacks = false)
public TestConnection(Func<Task> onStart = null, Func<Task> onDispose = null, bool autoHandshake = true, bool synchronousCallbacks = false)
{
_autoNegotiate = autoNegotiate;
_autoHandshake = autoHandshake;
_onStart = onStart ?? (() => Task.CompletedTask);
_onDispose = onDispose ?? (() => Task.CompletedTask);
@ -64,7 +64,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
await _onStart();
if (_autoNegotiate)
if (_autoHandshake)
{
// We can't await this as it will block StartAsync which will block
// HubConnection.StartAsync which sends the Handshake in the first place!