Skip to content
Permalink
Browse files
Explicitly close a document RTCNetworkManager on document teardown
https://bugs.webkit.org/show_bug.cgi?id=247413
rdar://99799545

Reviewed by Eric Carlson.

The combination of keeping a WeakPtr of a ThreadSafeRefCounted is problematic,
as we might have lost the last Ref/RefPtr (hence the object is scheduled to be deleted)
but we still have a non null reference via a WeakPtr, which we could potentially create a new Ref on it.
We fix the particular issue here by closing explicitly LibWebRTCNetworkManager
which will remove the LibWebRTCNetworkManager from the WebRTCMonitor WeakHashSet.
Add ASSERTs to ensure close is called before destroying LibWebRTCNetworkManager.

* Source/WebCore/Modules/mediastream/RTCNetworkManager.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::commonTeardown):
* Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetworkManager.cpp:
(WebKit::LibWebRTCNetworkManager::getOrCreate):
(WebKit::LibWebRTCNetworkManager::LibWebRTCNetworkManager):
(WebKit::LibWebRTCNetworkManager::~LibWebRTCNetworkManager):
(WebKit::LibWebRTCNetworkManager::close):

Canonical link: https://commits.webkit.org/256319@main
  • Loading branch information
youennf committed Nov 4, 2022
1 parent da064cb commit f394e0a05ff1d91c0909c4319c5fecd5e4e86b98
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 1 deletion.
@@ -33,6 +33,7 @@ namespace WebCore {
class RTCNetworkManager : public ThreadSafeRefCounted<RTCNetworkManager, WTF::DestructionThread::MainRunLoop> {
public:
virtual ~RTCNetworkManager() = default;
virtual void close() = 0;
virtual void setICECandidateFiltering(bool) = 0;
virtual void unregisterMDNSNames() = 0;
};
@@ -842,6 +842,13 @@ void Document::commonTeardown()
m_timeline = nullptr;
m_associatedFormControls.clear();
m_didAssociateFormControlsTimer.stop();

#if ENABLE(WEB_RTC)
if (m_rtcNetworkManager) {
m_rtcNetworkManager->close();
m_rtcNetworkManager = nullptr;
}
#endif
}

Element* Document::elementForAccessKey(const String& key)
@@ -48,6 +48,7 @@ LibWebRTCNetworkManager* LibWebRTCNetworkManager::getOrCreate(WebCore::ScriptExe
auto newNetworkManager = adoptRef(*new LibWebRTCNetworkManager(identifier));
networkManager = newNetworkManager.ptr();
document->setRTCNetworkManager(WTFMove(newNetworkManager));
WebProcess::singleton().libWebRTCNetwork().monitor().addObserver(*networkManager);
}

return networkManager;
@@ -56,11 +57,18 @@ LibWebRTCNetworkManager* LibWebRTCNetworkManager::getOrCreate(WebCore::ScriptExe
LibWebRTCNetworkManager::LibWebRTCNetworkManager(WebCore::ScriptExecutionContextIdentifier documentIdentifier)
: m_documentIdentifier(documentIdentifier)
{
WebProcess::singleton().libWebRTCNetwork().monitor().addObserver(*this);
}

LibWebRTCNetworkManager::~LibWebRTCNetworkManager()
{
ASSERT(m_isClosed);
}

void LibWebRTCNetworkManager::close()
{
#if ASSERT_ENABLED
m_isClosed = true;
#endif
WebProcess::singleton().libWebRTCNetwork().monitor().removeObserver(*this);
}

@@ -47,6 +47,7 @@ class LibWebRTCNetworkManager final : public WebCore::RTCNetworkManager, public
// WebCore::RTCNetworkManager
void setICECandidateFiltering(bool doFiltering) final { m_useMDNSCandidates = doFiltering; }
void unregisterMDNSNames() final;
void close() final;

// webrtc::NetworkManagerBase
void StartUpdating() final;
@@ -64,6 +65,9 @@ class LibWebRTCNetworkManager final : public WebCore::RTCNetworkManager, public
WebCore::ScriptExecutionContextIdentifier m_documentIdentifier;
bool m_useMDNSCandidates { true };
bool m_receivedNetworkList { false };
#if ASSERT_ENABLED
bool m_isClosed { false };
#endif
};

} // namespace WebKit

0 comments on commit f394e0a

Please sign in to comment.