Bug 1259728 - minimal fix for reentrancy in pc.close(). r=jesup
authorJan-Ivar Bruaroey <jib@mozilla.com>
Tue, 29 Mar 2016 16:27:03 -0400
changeset 291065 88df6d5a1f21bd9c8298f3503620a9647ef3d857
parent 291064 be61ede92adbcad7000db7ba128276ced1ce73b2
child 291066 3a587c25402101cc96761eaa7749b7a41f57461c
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs1259728
milestone48.0a1
Bug 1259728 - minimal fix for reentrancy in pc.close(). r=jesup MozReview-Commit-ID: Dji4d2bYTcj
dom/media/PeerConnection.js
dom/media/tests/mochitest/test_peerConnection_close.html
--- a/dom/media/PeerConnection.js
+++ b/dom/media/PeerConnection.js
@@ -617,18 +617,18 @@ RTCPeerConnection.prototype = {
     if (this._closed) {
       throw new this._win.DOMException("Peer connection is closed",
                                        "InvalidStateError");
     }
   },
 
   dispatchEvent: function(event) {
     // PC can close while events are firing if there is an async dispatch
-    // in c++ land
-    if (!this._closed) {
+    // in c++ land. But let through "closed" signaling and ice connection events.
+    if (!this._closed || this._inClose) {
       this.__DOM_IMPL__.dispatchEvent(event);
     }
   },
 
   // Log error message to web console and window.onerror, if present.
   logErrorAndCallOnError: function(e) {
     this.logMsg(e.message, e.fileName, e.lineNumber, Ci.nsIScriptError.exceptionFlag);
 
@@ -1134,21 +1134,23 @@ RTCPeerConnection.prototype = {
     }
     return this._impl.getParameters(sender.track);
   },
 
   close: function() {
     if (this._closed) {
       return;
     }
+    this._closed = true;
+    this._inClose = true;
     this.changeIceConnectionState("closed");
     this._localIdp.close();
     this._remoteIdp.close();
     this._impl.close();
-    this._closed = true;
+    this._inClose = false;
   },
 
   getLocalStreams: function() {
     this._checkClosed();
     return this._impl.getLocalStreams();
   },
 
   getRemoteStreams: function() {
--- a/dom/media/tests/mochitest/test_peerConnection_close.html
+++ b/dom/media/tests/mochitest/test_peerConnection_close.html
@@ -24,16 +24,26 @@
     var finish;
     var finished = new Promise(resolve => finish = resolve);
 
     pc.onsignalingstatechange = function(e) {
       clearTimeout(eTimeout);
       is(pc.signalingState, "closed", "signalingState is 'closed'");
       is(pc.iceConnectionState, "closed", "iceConnectionState is 'closed'");
 
+      // test that pc is really closed (and doesn't crash, bug 1259728)
+      try {
+        pc.getLocalStreams();
+      } catch (e) {
+        exception = e;
+      }
+      is(exception && exception.name, "InvalidStateError",
+         "pc.getLocalStreams should throw when closed");
+      exception = null;
+
       try {
         pc.close();
       } catch (e) {
         exception = e;
       }
       is(exception, null, "A second close() should not raise an exception");
       is(pc.signalingState, "closed", "Final signalingState stays at 'closed'");
       is(pc.iceConnectionState, "closed", "Final iceConnectionState stays at 'closed'");