Bug 955676 - Don't handle incoming data synchronously, r=clokep,florian.
authoraleth <aleth@instantbird.org>
Sat, 02 Nov 2013 12:57:14 -0400
changeset 19219 0ac2f96b0b12d5f2fd2644e5cdbe0e3c673f0e97
parent 19218 23e9bcfe799219fc9c5adc9efca4e7f0431ddeb9
child 19220 9f0bc5f86d2b9f8da78098fdf1698f62a29bee02
push id1103
push usermbanner@mozilla.com
push dateTue, 18 Mar 2014 07:44:06 +0000
treeherdercomm-beta@50c6279a0af0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersclokep, florian
bugs955676
Bug 955676 - Don't handle incoming data synchronously, r=clokep,florian.
chat/modules/socket.jsm
chat/protocols/irc/irc.js
--- a/chat/modules/socket.jsm
+++ b/chat/modules/socket.jsm
@@ -141,16 +141,19 @@ const Socket = {
   connect: function(aHost, aPort, aSecurity, aProxy) {
     if (Services.io.offline)
       throw Cr.NS_ERROR_FAILURE;
 
     this.LOG("Connecting to: " + aHost + ":" + aPort);
     this.host = aHost;
     this.port = aPort;
 
+    this._pendingData = [];
+    delete this._stopRequestStatus;
+
     // Array of security options
     this.security = aSecurity || [];
 
     // Choose a proxy, use the given one, otherwise get one from the proxy
     // service
     if (aProxy)
       this._createTransport(aProxy);
     else {
@@ -172,16 +175,19 @@ const Socket = {
       }
     }
   },
 
   // Disconnect all open streams.
   disconnect: function() {
     this.LOG("Disconnect");
 
+    // Don't handle any remaining unhandled data.
+    this._pendingData = [];
+
     // Close all input and output streams.
     if ("_inputStream" in this) {
       this._inputStream.close();
       delete this._inputStream;
     }
     if ("_outputStream" in this) {
       this._outputStream.close();
       delete this._outputStream;
@@ -198,16 +204,18 @@ const Socket = {
     }
 
     if (this._pingTimer) {
       clearTimeout(this._pingTimer);
       delete this._pingTimer;
       delete this._resetPingTimerPending;
     }
     this.cancelDisconnectTimer();
+
+    delete this.isConnected;
   },
 
   // Listen for a connection on a port.
   // XXX take a timeout and then call stopListening
   listen: function(port) {
     this.LOG("Listening on port " + port);
 
     this.serverSocket = new ServerSocket(port, false, -1);
@@ -353,16 +361,18 @@ const Socket = {
   },
 
   /*
    * nsIStreamListener methods
    */
   // onDataAvailable, called by Mozilla's networking code.
   // Buffers the data, and parses it into discrete messages.
   onDataAvailable: function(aRequest, aContext, aInputStream, aOffset, aCount) {
+    if (!this.isConnected)
+      return;
     if (this.binaryMode) {
       // Load the data from the stream
       this._incomingDataBuffer = this._incomingDataBuffer
                                      .concat(this._binaryInputStream
                                                  .readByteArray(aCount));
 
       let size = this._incomingDataBuffer.length;
 
@@ -374,44 +384,89 @@ const Socket = {
       uintArray.set(this._incomingDataBuffer);
 
       // Notify we've received data.
       // Variable data size, the callee must return how much data was handled.
       size = this.onBinaryDataReceived(buffer);
 
       // Remove the handled data.
       this._incomingDataBuffer.splice(0, size);
-    } else {
+    }
+    else {
       if (this.delimiter) {
-        // Load the data from the stream
+        // Load the data from the stream.
         this._incomingDataBuffer += this._scriptableInputStream.read(aCount);
         let data = this._incomingDataBuffer.split(this.delimiter);
 
-        // Store the (possibly) incomplete part
+        // Store the (possibly) incomplete part.
         this._incomingDataBuffer = data.pop();
+        if (!data.length)
+          return;
+
+        // Add the strings to the queue.
+        this._pendingData = this._pendingData.concat(data);
+      }
+      else {
+        // Add the whole string to the queue.
+        this._pendingData.push(this._scriptableInputStream.read(aCount));
+      }
+      this._activateQueue();
+    }
+  },
 
-        // Send each string to the handle data function
-        data.forEach(this.onDataReceived, this);
-      } else {
-        // Send the whole string to the handle data function
-        this.onDataReceived(this._scriptableInputStream.read(aCount));
-      }
+  _pendingData: [],
+  _handlingQueue: false,
+  _activateQueue: function() {
+    if (this._handlingQueue)
+      return;
+    this._handlingQueue = true;
+    this._handleQueue();
+  },
+  // Asynchronously send each string to the handle data function.
+  _handleQueue: function() {
+    let begin = Date.now();
+    while (this._pendingData.length) {
+      this.onDataReceived(this._pendingData.shift());
+      // If more than 10ms have passed, stop blocking the thread.
+      if (Date.now() - begin > 10)
+        break;
     }
+    if (this._pendingData.length) {
+      executeSoon(this._handleQueue.bind(this));
+      return;
+    }
+    delete this._handlingQueue;
+    // If there was a stop request, handle it.
+    if ("_stopRequestStatus" in this)
+      this._handleStopRequest(this._stopRequestStatus);
   },
 
   /*
    * nsIRequestObserver methods
    */
   // Signifies the beginning of an async request
   onStartRequest: function(aRequest, aContext) {
     this.DEBUG("onStartRequest");
   },
   // Called to signify the end of an asynchronous request.
   onStopRequest: function(aRequest, aContext, aStatus) {
+    if (!this.isConnected) {
+      // We're already disconnected, so nothing left to do here.
+      return;
+    }
+
     this.DEBUG("onStopRequest (" + aStatus + ")");
+    this._stopRequestStatus = aStatus;
+    // The stop request will be handled when the queue is next empty.
+    this._activateQueue();
+  },
+  // Close the connection after receiving a stop request.
+  _handleStopRequest: function(aStatus) {
+    if (!this.isConnected)
+      return;
     delete this.isConnected;
     if (aStatus == NS_ERROR_NET_RESET)
       this.onConnectionReset();
     else if (aStatus == NS_ERROR_NET_TIMEOUT)
       this.onConnectionTimedOut();
     else if (!Components.isSuccessCode(aStatus)) {
       let nssErrorsService =
         Cc["@mozilla.org/nss_errors_service;1"].getService(Ci.nsINSSErrorsService);
--- a/chat/protocols/irc/irc.js
+++ b/chat/protocols/irc/irc.js
@@ -701,25 +701,33 @@ ircSocket.prototype = {
   },
   onConnection: function() {
     this._account._connectionRegistration.call(this._account);
   },
   disconnect: function() {
     if (!this._account)
       return;
     Socket.disconnect.call(this);
-    this.onStopRequest = function() {};
     delete this._account;
   },
 
   // Throw errors if the socket has issues.
   onConnectionClosed: function() {
-    this.ERROR("Connection closed by server.");
-    this._account.gotDisconnected(Ci.prplIAccount.ERROR_NETWORK_ERROR,
-                                  _("connection.error.lost"));
+    const msg = "Connection closed by server.";
+    if (this._account.disconnecting) {
+      // The server closed the connection before we handled the ERROR
+      // response to QUIT.
+      this.LOG(msg);
+      this._account.gotDisconnected();
+    }
+    else {
+      this.ERROR(msg);
+      this._account.gotDisconnected(Ci.prplIAccount.ERROR_NETWORK_ERROR,
+                                    _("connection.error.lost"));
+    }
   },
   onConnectionReset: function() {
     this.ERROR("Connection reset.");
     this._account.gotDisconnected(Ci.prplIAccount.ERROR_NETWORK_ERROR,
                                   _("connection.error.lost"));
   },
   onConnectionTimedOut: function() {
     this.ERROR("Connection timed out.");