Skip to content

Commit

Permalink
Require the WebSocket Transport make an /abort request to abort
Browse files Browse the repository at this point in the history
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
  • Loading branch information
halter73 committed Sep 20, 2013
1 parent 7f97baf commit a3bd526
Show file tree
Hide file tree
Showing 12 changed files with 195 additions and 150 deletions.
Expand Up @@ -125,7 +125,8 @@
}
},

abort: function (connection) {
abort: function (connection, async) {
transportLogic.ajaxAbort(connection, async);
}
};

Expand Down
Expand Up @@ -118,6 +118,9 @@
<Link>Transports\TransportHelper.cs</Link>
</Compile>
<Compile Include="Properties\AssemblyInfo.cs" />
<Compile Include="..\Microsoft.AspNet.SignalR.Client\Infrastructure\TransportAbortHandler.cs">
<Link>Infrastructure\TransportAbortHandler.cs</Link>
</Compile>
</ItemGroup>
<ItemGroup>
<Compile Include="..\Common\CommonAssemblyInfo.cs">
Expand Down
Expand Up @@ -194,6 +194,9 @@
<Compile Include="..\Microsoft.AspNet.SignalR.Client\Infrastructure\ExceptionHelper.cs">
<Link>Infrastructure\ExceptionHelper.cs</Link>
</Compile>
<Compile Include="..\Microsoft.AspNet.SignalR.Client\Infrastructure\TransportAbortHandler.cs">
<Link>Infrastructure\TransportAbortHandler.cs</Link>
</Compile>
<Compile Include="..\Microsoft.AspNet.SignalR.Client\Infrastructure\StreamExtensions.cs">
<Link>Infrastructure\StreamExtensions.cs</Link>
</Compile>
Expand Down
Expand Up @@ -123,6 +123,9 @@
<Compile Include="..\Microsoft.AspNet.SignalR.Client\Http\IResponseExtensions.cs">
<Link>Http\IResponseExtensions.cs</Link>
</Compile>
<Compile Include="..\Microsoft.AspNet.SignalR.Client\Infrastructure\TransportAbortHandler.cs">
<Link>Infrastructure\TransportAbortHandler.cs</Link>
</Compile>
<Compile Include="..\Microsoft.AspNet.SignalR.Client\Resources.Designer.cs">
<Link>Resources.Designer.cs</Link>
</Compile>
Expand Down
@@ -0,0 +1,139 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Text;
using System.Threading;
using Microsoft.AspNet.SignalR.Client.Http;
using Microsoft.AspNet.SignalR.Client.Transports;

namespace Microsoft.AspNet.SignalR.Client.Infrastructure
{
public sealed class TransportAbortHandler : IDisposable
{
// The abort query string
private const string _abortQueryString = "?transport={0}&connectionToken={1}{2}";

// The transport name
private readonly string _transportName;

private readonly IHttpClient _httpClient;

// Used to complete the synchronous call to Abort()
private ManualResetEvent _abortResetEvent = new ManualResetEvent(initialState: false);

// Used to indicate whether Abort() has been called
private bool _startedAbort;
// Used to ensure that Abort() runs effectively only once
// The _abortLock subsumes the _disposeLock and can be held upwards of 30 seconds
private readonly object _abortLock = new object();

// Used to ensure the _abortResetEvent.Set() isn't called after disposal
private bool _disposed;
// Used to make checking _disposed and calling _abortResetEvent.Set() thread safe
private readonly object _disposeLock = new object();

public TransportAbortHandler(IHttpClient httpClient, string transportName)
{
_httpClient = httpClient;
_transportName = transportName;
}

public void Abort(IConnection connection, TimeSpan timeout)
{
if (connection == null)
{
throw new ArgumentNullException("connection");
}

// Abort should never complete before any of its previous calls
lock (_abortLock)
{
if (_disposed)
{
throw new ObjectDisposedException(GetType().Name);
}

// Ensure that an abort request is only made once
if (!_startedAbort)
{
_startedAbort = true;

string url = connection.Url + "abort" + String.Format(CultureInfo.InvariantCulture,
_abortQueryString,
_transportName,
Uri.EscapeDataString(connection.ConnectionToken),
null);

url += TransportHelper.AppendCustomQueryString(connection, url);

_httpClient.Post(url, connection.PrepareRequest, isLongRunning: false).Catch((ex, state) =>
{
// If there's an error making an http request set the reset event
((TransportAbortHandler)state).CompleteAbort();
},
this);

if (!_abortResetEvent.WaitOne(timeout))
{
connection.Trace(TraceLevels.Events, "Abort never fired");
}
}
}
}

public void CompleteAbort()
{
lock (_disposeLock)
{
if (!_disposed)
{
// Make any future calls to Abort() no-op
// Abort might still run, but any ongoing aborts will immediately complete
_startedAbort = true;
// Ensure any ongoing calls to Abort() complete
_abortResetEvent.Set();
}
}
}

public bool TryCompleteAbort()
{
// Make sure we don't Set a disposed ManualResetEvent
lock (_disposeLock)
{
if (_disposed)
{
// Don't try to continue receiving messages if the transport is disposed
return true;
}
else if (_startedAbort)
{
_abortResetEvent.Set();
return true;
}
else
{
return false;
}
}
}

public void Dispose()
{
// Wait for any ongoing aborts to complete
// In practice, any aborts should have finished by the time Dispose is called
lock (_abortLock)
{
lock (_disposeLock)
{
if (!_disposed)
{
_abortResetEvent.Dispose();
_disposed = true;
}
}
}
}
}
}
Expand Up @@ -81,6 +81,7 @@
<Compile Include="Hubs\Subscription.cs" />
<Compile Include="IConnection.cs" />
<Compile Include="Infrastructure\ErrorExtensions.cs" />
<Compile Include="Infrastructure\TransportAbortHandler.cs" />
<Compile Include="KeepAliveData.cs" />
<Compile Include="Infrastructure\SignalRError.cs" />
<Compile Include="Infrastructure\ExceptionHelper.cs" />
Expand Down
111 changes: 10 additions & 101 deletions src/Microsoft.AspNet.SignalR.Client/Transports/HttpBasedTransport.cs
Expand Up @@ -7,6 +7,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNet.SignalR.Client.Http;
using Microsoft.AspNet.SignalR.Client.Infrastructure;
using Microsoft.AspNet.SignalR.Infrastructure;
using Newtonsoft.Json.Linq;

Expand All @@ -20,26 +21,14 @@ public abstract class HttpBasedTransport : IClientTransport
// The transport name
private readonly string _transport;

// Used to complete the synchronous call to Abort()
private ManualResetEvent _abortResetEvent = new ManualResetEvent(initialState: false);

// Used to indicate whether Abort() has been called
private bool _startedAbort;
// Used to ensure that Abort() runs effectively only once
// The _abortLock subsumes the _disposeLock and can be held upwards of 30 seconds
private readonly object _abortLock = new object();

// Used to ensure the _abortResetEvent.Set() isn't called after disposal
private bool _disposed;
// Used to make checking _disposed and calling _abortResetEvent.Set() thread safe
private readonly object _disposeLock = new object();

private readonly IHttpClient _httpClient;
private readonly TransportAbortHandler _abortHandler;

protected HttpBasedTransport(IHttpClient httpClient, string transport)
{
_httpClient = httpClient;
_transport = transport;
_abortHandler = new TransportAbortHandler(httpClient, transport);
}

public string Name
Expand All @@ -60,6 +49,11 @@ protected IHttpClient HttpClient
get { return _httpClient; }
}

protected TransportAbortHandler AbortHandler
{
get { return _abortHandler; }
}

public Task<NegotiationResponse> Negotiate(IConnection connection)
{
return _httpClient.GetNegotiationResponse(connection);
Expand Down Expand Up @@ -116,82 +110,7 @@ public Task Send(IConnection connection, string data)

public void Abort(IConnection connection, TimeSpan timeout)
{
if (connection == null)
{
throw new ArgumentNullException("connection");
}

// Abort should never complete before any of its previous calls
lock (_abortLock)
{
if (_disposed)
{
throw new ObjectDisposedException(GetType().Name);
}

// Ensure that an abort request is only made once
if (!_startedAbort)
{
_startedAbort = true;

string url = connection.Url + "abort" + String.Format(CultureInfo.InvariantCulture,
_sendQueryString,
_transport,
Uri.EscapeDataString(connection.ConnectionToken),
null);

url += TransportHelper.AppendCustomQueryString(connection, url);

_httpClient.Post(url, connection.PrepareRequest, isLongRunning: false).Catch((ex, state) =>
{
// If there's an error making an http request set the reset event
((HttpBasedTransport)state).CompleteAbort();
},
this);

if (!_abortResetEvent.WaitOne(timeout))
{
connection.Trace(TraceLevels.Events, "Abort never fired");
}
}
}
}

protected void CompleteAbort()
{
lock (_disposeLock)
{
if (!_disposed)
{
// Make any future calls to Abort() no-op
// Abort might still run, but any ongoing aborts will immediately complete
_startedAbort = true;
// Ensure any ongoing calls to Abort() complete
_abortResetEvent.Set();
}
}
}

protected bool TryCompleteAbort()
{
// Make sure we don't Set a disposed ManualResetEvent
lock (_disposeLock)
{
if (_disposed)
{
// Don't try to continue receiving messages if the transport is disposed
return true;
}
else if (_startedAbort)
{
_abortResetEvent.Set();
return true;
}
else
{
return false;
}
}
_abortHandler.Abort(connection, timeout);
}

protected string GetReceiveQueryString(IConnection connection, string data)
Expand All @@ -211,17 +130,7 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
// Wait for any ongoing aborts to complete
// In practice, any aborts should have finished by the time Dispose is called
lock (_abortLock)
lock (_disposeLock)
{
if (!_disposed)
{
_abortResetEvent.Dispose();
_disposed = true;
}
}
_abortHandler.Dispose();
}
}
}
Expand Down
Expand Up @@ -189,7 +189,7 @@ public override bool SupportsKeepAlive
{
requestDisposer.Dispose();
if (TryCompleteAbort())
if (AbortHandler.TryCompleteAbort())
{
// Abort() was called, so don't reconnect
requestHandler.Stop();
Expand All @@ -213,7 +213,7 @@ public override bool SupportsKeepAlive
{
// Complete any ongoing calls to Abort()
// If someone calls Abort() later, have it no-op
CompleteAbort();
AbortHandler.CompleteAbort();
};
}

Expand Down
Expand Up @@ -188,9 +188,9 @@ private void Reconnect(IConnection connection, string data, CancellationToken di
if (stop)
{
CompleteAbort();
AbortHandler.CompleteAbort();
}
else if (TryCompleteAbort())
else if (AbortHandler.TryCompleteAbort())
{
// Abort() was called, so don't reconnect
}
Expand Down
Expand Up @@ -124,6 +124,9 @@
<Compile Include="..\Microsoft.AspNet.SignalR.Client\IConnection.cs">
<Link>IConnection.cs</Link>
</Compile>
<Compile Include="..\Microsoft.AspNet.SignalR.Client\Infrastructure\TransportAbortHandler.cs">
<Link>Infrastructure\TransportAbortHandler.cs</Link>
</Compile>
<Compile Include="..\Microsoft.AspNet.SignalR.Client\KeepAliveData.cs">
<Link>KeepAliveData.cs</Link>
</Compile>
Expand Down

0 comments on commit a3bd526

Please sign in to comment.