Fixed more issues with disconnect and clean disconnect.

- Made a new abort command so disconnect events aren't fired multiple times for the same client. When a clean disconnect is triggered, the client will send an abort command and the receiving end will only raise disconnect for those commands, instead of for the disconnect command. The disconnect command signifies an unclean disconnect and therefore no other event needs to be raised. Remove connections from the list of tracked connections when an abort command is received. This prevents the disconnect event to fire again for that connection. - Fixed a nasty issue with the longpolling transport in the javascript client sending requests forever when disconnect commands are received.
SignalR · May 18, 2012 · 3e64c0e · 3e64c0e
1 parent 08fe8c2
commit 3e64c0e
Show file tree

Hide file tree

Showing 16 changed files with 111 additions and 28 deletions.
diff --git a/SignalR/CommandType.cs b/SignalR/CommandType.cs
@@ -4,6 +4,7 @@ public enum CommandType
     {
         AddToGroup,
         RemoveFromGroup,
-        Disconnect
+        Disconnect,
+        Abort
     }
 }
diff --git a/SignalR/Connection.cs b/SignalR/Connection.cs
@@ -18,6 +18,7 @@ public class Connection : IConnection, ITransportConnection
         private readonly HashSet<string> _groups;
         private readonly ITraceManager _trace;
         private bool _disconnected;
+        private bool _aborted;
 
         public Connection(IMessageBus messageBus,
                           IJsonSerializer jsonSerializer,
@@ -87,6 +88,7 @@ private PersistentResponse GetResponse(MessageResult result)
                 MessageId = result.LastMessageId,
                 Messages = messageValues,
                 Disconnect = _disconnected,
+                Aborted = _aborted,
                 TimedOut = result.TimedOut
             };
 
@@ -133,6 +135,9 @@ private void ProcessCommand(SignalCommand command)
                 case CommandType.Disconnect:
                     _disconnected = true;
                     break;
+                case CommandType.Abort:
+                    _aborted = true;
+                    break;
             }
         }
 

diff --git a/SignalR/Infrastructure/ConnectionExtensions.cs b/SignalR/Infrastructure/ConnectionExtensions.cs
@@ -15,6 +15,16 @@ public static Task Close(this ITransportConnection connection)
             return connection.SendCommand(command);
         }
 
+        public static Task Abort(this ITransportConnection connection)
+        {
+            var command = new SignalCommand
+            {
+                Type = CommandType.Abort
+            };
+
+            return connection.SendCommand(command);
+        }
+
         public static Task SendCommand(this IConnection connection, string connectionId, SignalCommand command)
         {
             return connection.Send(SignalCommand.AddCommandSuffix(connectionId), command);

diff --git a/SignalR/PersistentResponse.cs b/SignalR/PersistentResponse.cs
@@ -1,4 +1,5 @@
 using System.Collections.Generic;
+using Newtonsoft.Json;
 
 namespace SignalR
 {
@@ -24,6 +25,12 @@ public class PersistentResponse
         /// </summary>
         public bool Disconnect { get; set; }
 
+        /// <summary>
+        /// True if the connection was forcibly closed. 
+        /// </summary>
+        [JsonIgnore]
+        public bool Aborted { get; set; }
+
         /// <summary>
         /// True if the connection timed out.
         /// </summary>

diff --git a/SignalR/Scripts/jquery.signalR.js b/SignalR/Scripts/jquery.signalR.js
@@ -422,7 +422,7 @@
                     log("Disconnect command received from server", connection.logging);
 
                     // Disconnected by the server
-                    connection.transport.stop(connection);
+                    connection.stop();
                     return;
                 }
 
@@ -837,7 +837,9 @@
             start: function (connection, onSuccess, onFailed) {
                 /// <summary>Starts the long polling connection</summary>
                 /// <param name="connection" type="signalR">The SignalR connection to start</param>
-                var that = this;
+                var that = this,
+                    initialConnectFired = false;
+
                 if (connection.pollXhr) {
                     log("Polling xhr requests already exists, aborting.");
                     connection.stop();
@@ -865,6 +867,11 @@
                                 var delay = 0,
                                     timedOutReceived = false;
 
+                                if (initialConnectFired == false) {
+                                    onSuccess();
+                                    initialConnectFired = true;
+                                }
+
                                 if (raiseReconnect === true) {
                                     // Fire the reconnect event if it hasn't been fired as yet
                                     if (reconnectFired === false) {
@@ -885,6 +892,10 @@
                                     timedOutReceived = data.TimedOut;
                                 }
 
+                                if (data && data.Disconnect) {
+                                    return;
+                                }
+
                                 if (delay > 0) {
                                     window.setTimeout(function () {
                                         poll(instance, timedOutReceived);
@@ -931,7 +942,12 @@
                     // Now connected
                     // There's no good way know when the long poll has actually started so
                     // we assume it only takes around 150ms (max) to start the connection
-                    window.setTimeout(onSuccess, 150);
+                    window.setTimeout(function () {
+                        if (initialConnectFired === false) {
+                            onSuccess();
+                            initialConnectFired = true;
+                        }
+                    }, 150);
 
                 }, 250); // Have to delay initial poll so Chrome doesn't show loader spinner in tab
             },

diff --git a/SignalR/Scripts/jquery.signalR.min.js b/SignalR/Scripts/jquery.signalR.min.js
diff --git a/SignalR/Transports/ForeverTransport.cs b/SignalR/Transports/ForeverTransport.cs
@@ -87,7 +87,7 @@ protected Task ProcessRequestCore(ITransportConnection connection)
             }
             else if (IsAbortRequest)
             {
-                return Connection.Close();
+                return Connection.Abort();
             }
             else
             {
@@ -201,10 +201,11 @@ private void ProcessMessagesImpl(TaskCompletionSource<object> taskCompletetionSo
                     // If the response has the Disconnect flag, just send the response and exit the loop,
                     // the server thinks connection is gone. Otherwse, send the response then re-enter the loop
                     Task sendTask = Send(response);
-                    if (response.Disconnect || response.TimedOut)
+                    if (response.Disconnect || response.TimedOut || response.Aborted)
                     {
-                        if (response.Disconnect)
+                        if (response.Aborted)
                         {
+                            // If this was a clean disconnect raise the event.
                             OnDisconnect();
                         }
 
@@ -232,11 +233,6 @@ private void ProcessMessagesImpl(TaskCompletionSource<object> taskCompletetionSo
                 return;
             }
 
-            if (!IsTimedOut)
-            {
-                OnDisconnect();
-            }
-
             taskCompletetionSource.SetResult(null);
             return;
         }

diff --git a/SignalR/Transports/ITransportHeartBeat.cs b/SignalR/Transports/ITransportHeartBeat.cs
@@ -22,5 +22,11 @@ public interface ITransportHeartBeat
         /// </summary>
         /// <param name="connection">The connection to mark.</param>
         void MarkConnection(ITrackingConnection connection);
+
+        /// <summary>
+        /// Removes a connection from the list of tracked connections.
+        /// </summary>
+        /// <param name="connection">The connection to remove.</param>
+        void RemoveConnection(ITrackingConnection connection);
     }
 }
diff --git a/SignalR/Transports/LongPollingTransport.cs b/SignalR/Transports/LongPollingTransport.cs
@@ -112,7 +112,7 @@ public Task ProcessRequest(ITransportConnection connection)
             }
             else if (IsAbortRequest)
             {
-                return Connection.Close();
+                return Connection.Abort();
             }
             else
             {
@@ -213,8 +213,9 @@ private Task ProcessReceiveRequest(ITransportConnection connection, Action postR
 
             return receiveTask.Then(response =>
             {
-                if (response.Disconnect)
+                if (response.Aborted)
                 {
+                    // If this was a clean disconnect then raise the event
                     OnDisconnect();
                 }
 

diff --git a/SignalR/Transports/TransportDisconnectBase.cs b/SignalR/Transports/TransportDisconnectBase.cs
@@ -120,6 +120,10 @@ public Task Disconnect()
 
         public Task OnDisconnect()
         {
+            // When a connection is aborted (graceful disconnect) we send a command to it
+            // telling to to disconnect. At that moment, we raise the disconnect event and
+            // remove this connection from the heartbeat so we don't end up raising it for the same connection.
+            HeartBeat.RemoveConnection(this);
             if (Interlocked.Exchange(ref _isDisconnected, 1) == 0)
             {
                 var disconnected = Disconnected; // copy before invoking event to avoid race

diff --git a/SignalR/Transports/TransportHeartBeat.cs b/SignalR/Transports/TransportHeartBeat.cs
@@ -80,7 +80,11 @@ private void RemoveConnection(string connectionId)
             RemoveConnection(new ConnectionReference(connectionId));
         }
 
-        private void RemoveConnection(ITrackingConnection connection)
+        /// <summary>
+        /// Removes a connection from the list of tracked connections.
+        /// </summary>
+        /// <param name="connection">The connection to remove.</param>
+        public void RemoveConnection(ITrackingConnection connection)
         {
             // Remove the connection and associated metadata
             _connections.Remove(connection);

diff --git a/samples/SignalR.Hosting.AspNet.Samples/Raw/index.htm b/samples/SignalR.Hosting.AspNet.Samples/Raw/index.htm
@@ -46,11 +46,15 @@
                           .appendTo($("#messages"));
             });
 
+            connection.disconnected(function () {
+                $("#stopStart").val("Start")
+                               .prop("disabled", false);
+            });
 
             var start = function () {
                 connection.start({ transport: activeTransport })
                     .then(function () {
-                        $("#stopStart").prop("disabled", false);
+                        $("#stopStart").val("Stop").prop("disabled", false);
                     });
             };
             start();
@@ -88,11 +92,8 @@
                 $el.prop("disabled", true);
                 if ($el.val() === "Stop") {
                     connection.stop();
-                    $el.val("Start")
-                       .prop("disabled", false);
                 } else {
                     start();
-                    $el.val("Stop");
                 }
             });
         });

diff --git a/samples/SignalR.Hosting.AspNet.Samples/Scripts/jquery.signalR.js b/samples/SignalR.Hosting.AspNet.Samples/Scripts/jquery.signalR.js
@@ -422,7 +422,7 @@
                     log("Disconnect command received from server", connection.logging);
 
                     // Disconnected by the server
-                    connection.transport.stop(connection);
+                    connection.stop();
                     return;
                 }
 
@@ -837,7 +837,9 @@
             start: function (connection, onSuccess, onFailed) {
                 /// <summary>Starts the long polling connection</summary>
                 /// <param name="connection" type="signalR">The SignalR connection to start</param>
-                var that = this;
+                var that = this,
+                    initialConnectFired = false;
+
                 if (connection.pollXhr) {
                     log("Polling xhr requests already exists, aborting.");
                     connection.stop();
@@ -865,6 +867,11 @@
                                 var delay = 0,
                                     timedOutReceived = false;
 
+                                if (initialConnectFired == false) {
+                                    onSuccess();
+                                    initialConnectFired = true;
+                                }
+
                                 if (raiseReconnect === true) {
                                     // Fire the reconnect event if it hasn't been fired as yet
                                     if (reconnectFired === false) {
@@ -885,6 +892,10 @@
                                     timedOutReceived = data.TimedOut;
                                 }
 
+                                if (data && data.Disconnect) {
+                                    return;
+                                }
+
                                 if (delay > 0) {
                                     window.setTimeout(function () {
                                         poll(instance, timedOutReceived);
@@ -931,7 +942,12 @@
                     // Now connected
                     // There's no good way know when the long poll has actually started so
                     // we assume it only takes around 150ms (max) to start the connection
-                    window.setTimeout(onSuccess, 150);
+                    window.setTimeout(function () {
+                        if (initialConnectFired === false) {
+                            onSuccess();
+                            initialConnectFired = true;
+                        }
+                    }, 150);
 
                 }, 250); // Have to delay initial poll so Chrome doesn't show loader spinner in tab
             },