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 291006 88df6d5a1f21bd9c8298f3503620a9647ef3d857
parent 291005 be61ede92adbcad7000db7ba128276ced1ce73b2
child 291007 3a587c25402101cc96761eaa7749b7a41f57461c
push id30128
push userkwierso@gmail.com
push dateThu, 31 Mar 2016 20:04:34 +0000
treeherdermozilla-central@bccb11375f2a [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'");