Skip to content

Commit

Permalink
Added an indicator to the OnClose to determine if
Browse files Browse the repository at this point in the history 
has been a clean/unclean close.  Also added the same changes to the
WebSocketHandler and DefaultWebSocketHandler for consistencies sake.

#735
  • Loading branch information
@NTaylorMullen
NTaylorMullen committed Oct 4, 2012
1 parent d75b17c commit 6d03262
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 59 deletions.
28 changes: 4 additions & 24 deletions SignalR.Hosting.AspNet45/WebSockets/DefaultWebSocketHandler.cs
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -7,31 +7,17 @@ internal class DefaultWebSocketHandler : WebSocketHandler, IWebSocket
{ {
private bool _raiseEvent = true; private bool _raiseEvent = true;


public override void OnClose() public override void OnClose(bool clean)
{ {
if (!_raiseEvent) if (!_raiseEvent)
{ {
return; return;
} }


Action onClose = ((IWebSocket)this).OnClose; Action<bool> onClose = ((IWebSocket)this).OnClose;
if (onClose != null) if (onClose != null)
{ {
onClose(); onClose(clean);
}
}

public override void OnUngracefulClose()
{
if (!_raiseEvent)
{
return;
}

Action onUngracefulClose = ((IWebSocket)this).OnUngracefulClose;
if (onUngracefulClose != null)
{
onUngracefulClose();
} }
} }


Expand Down Expand Up @@ -65,13 +51,7 @@ Action<string> IWebSocket.OnMessage
set; set;
} }


Action IWebSocket.OnClose Action<bool> IWebSocket.OnClose
{
get;
set;
}

Action IWebSocket.OnUngracefulClose
{ {
get; get;
set; set;
Expand Down
21 changes: 5 additions & 16 deletions SignalR.Hosting.AspNet45/WebSockets/WebSocketHandler.cs
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -32,11 +32,8 @@ public virtual void OnOpen() { }
// The developer can look at the Error property to get the exception // The developer can look at the Error property to get the exception
public virtual void OnError() { } public virtual void OnError() { }


// Called when the socket is closed gracefully; invoked 1 time per socket // Called when the socket is closed; invoked 1 time per socket
public virtual void OnClose() { } public virtual void OnClose(bool clean) { }

// Called when the socket is closed ungracefully
public virtual void OnUngracefulClose() { }


/* /*
* NON-VIRTUAL HELPER METHODS * NON-VIRTUAL HELPER METHODS
Expand Down Expand Up @@ -130,7 +127,7 @@ public Task ProcessWebSocketRequestAsync(WebSocketContext webSocketContext)


internal async Task ProcessWebSocketRequestAsync(WebSocketContext webSocketContext, Func<Task<WebSocketMessage>> messageRetriever) internal async Task ProcessWebSocketRequestAsync(WebSocketContext webSocketContext, Func<Task<WebSocketMessage>> messageRetriever)
{ {
bool GracefulClose = true; bool CleanClose = true;
try try
{ {
// first, set primitives and initialize the object // first, set primitives and initialize the object
Expand Down Expand Up @@ -167,22 +164,14 @@ await Task.WhenAny(CloseAsync(), Task.Delay(_closeTimeout))
{ {
Error = ex; Error = ex;
OnError(); OnError();
GracefulClose = false; CleanClose = false;
} }
} }
finally finally
{ {
try try
{ {
if (GracefulClose) OnClose(CleanClose);
{
// we're finished
OnClose();
}
else
{
OnUngracefulClose();
}
} }
finally finally
{ {
Expand Down
10 changes: 5 additions & 5 deletions SignalR.Server/ServerRequestWebSocket.cs
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ public Task Invoke(IDictionary<string, object> context)
} }


public Action<string> OnMessage { get; set; } public Action<string> OnMessage { get; set; }
public Action OnClose { get; set; } public Action<bool> OnClose { get; set; }
public Action OnUngracefulClose { get; set; }
public Action<Exception> OnError { get; set; } public Action<Exception> OnError { get; set; }


public Task Send(string value) public Task Send(string value)
Expand Down Expand Up @@ -163,7 +162,7 @@ private void ContinueReceiving(WebSocketReceiveTuple receiveData)
{ {
if (OnClose != null) if (OnClose != null)
{ {
OnClose(); OnClose(true);
} }


return; return;
Expand Down Expand Up @@ -192,11 +191,12 @@ private void ErrorReceiving(Exception error)
} }
} }


if (OnUngracefulClose != null) // Unclean disconnect
if (OnClose != null)
{ {
try try
{ {
OnUngracefulClose(); OnClose(false);
} }
catch catch
{ {
Expand Down
7 changes: 1 addition & 6 deletions SignalR/Hosting/IWebSocket.cs
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@ public interface IWebSocket
/// <summary> /// <summary>
/// Invoked when the websocket gracefully closes /// Invoked when the websocket gracefully closes
/// </summary> /// </summary>
Action OnClose { get; set; } Action<bool> OnClose { get; set; }

/// <summary>
/// Invoked when the websocket ungracefully closes
/// </summary>
Action OnUngracefulClose { get; set; }


/// <summary> /// <summary>
/// Invoked when there is an error /// Invoked when there is an error
Expand Down
13 changes: 5 additions & 8 deletions SignalR/Transports/WebSocketTransport.cs
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -47,16 +47,13 @@ public override Task ProcessRequest(ITransportConnection connection)
{ {
_socket = socket; _socket = socket;
socket.OnClose = () => socket.OnClose = clean =>
{ {
OnDisconnect(); if (clean)
{
_isAlive = false; OnDisconnect();
}; }
socket.OnUngracefulClose = () =>
{
// Die but let the heartbeat clean up the connection
_isAlive = false; _isAlive = false;
}; };
Expand Down

4 comments on commit 6d03262

@davidfowl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if it's an unclean disconnect Disconnect never fires? What's the idea behind that?

@NTaylorMullen
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's an unclean disconnect we need to leave it for the Heartbeat to clean up. How it worked in the past is that it would disconnect and therefore be removed from the heartbeat before it was considered "timed out".

All of the other transports get cleaned up by the heartbeat when the client goes away. By not calling disconnect on an unclean disconnect we now make websockets like the other transports.

A pro to doing it this way is that if a client disconnects due to a network drop they are then allotted the 110 second (current config value) timeout period to reconnect to the server before their connection is removed from the connection pool.

@davidfowl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some comments to the code?

@NTaylorMullen
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doneski! fcd1641

Please sign in to comment.