Skip to content

Commit

Permalink
Bug 1755318: Move the RemovePeerConnection call to Close. r=mjf
Browse files Browse the repository at this point in the history
This avoids the situation where PeerConnectionCtx is holding onto bare pointers
to PeerConnectionImpl whose refcounts have gone to 0, which is a pretty big
footgun.

Differential Revision: https://phabricator.services.mozilla.com/D139214
  • Loading branch information
docfaraday committed Mar 1, 2022
1 parent 4456c36 commit f666527
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions dom/media/webrtc/jsapi/PeerConnectionImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -384,12 +384,6 @@ PeerConnectionImpl::~PeerConnectionImpl() {
mTimeCard = nullptr;
}

if (PeerConnectionCtx::isActive()) {
PeerConnectionCtx::GetInstance()->RemovePeerConnection(mHandle);
} else {
CSFLogError(LOGTAG, "PeerConnectionCtx is already gone. Ignoring...");
}

CSFLogInfo(LOGTAG, "%s: PeerConnectionImpl destructor invoked for %s",
__FUNCTION__, mHandle.c_str());
}
Expand Down Expand Up @@ -1891,6 +1885,7 @@ PeerConnectionImpl::Close() {
return NS_OK;
}

STAMP_TIMECARD(mTimeCard, "Close");
mSignalingState = RTCSignalingState::Closed;

// When ICE completes, we record a bunch of statistics that outlive the
Expand Down Expand Up @@ -1999,6 +1994,12 @@ PeerConnectionImpl::Close() {
}
});

if (PeerConnectionCtx::isActive()) {
// If we're shutting down xpcom, this Instance will be unset before calling
// Close() on all remaining PCs, to avoid reentrancy.
PeerConnectionCtx::GetInstance()->RemovePeerConnection(mHandle);
}

return NS_OK;
}

Expand Down

0 comments on commit f666527

Please sign in to comment.