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 290888 88df6d5a1f21bd9c8298f3503620a9647ef3d857
parent 290887 be61ede92adbcad7000db7ba128276ced1ce73b2
child 290889 3a587c25402101cc96761eaa7749b7a41f57461c
push id74412
push userjbruaroey@mozilla.com
push dateWed, 30 Mar 2016 12:35:51 +0000
treeherdermozilla-inbound@88df6d5a1f21 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs1259728
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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'");