Skip to content

Commit

Permalink
DNSServiceRef can outlive its socket connection to mDNS, causing bloc…
Browse files Browse the repository at this point in the history
…ked threads in Networking

rdar://112721318
https://bugs.webkit.org/show_bug.cgi?id=259636

Reviewed by Youenn Fablet.

Be sure to tear down the DNSServiceRef and remove it from our set when error conditions occur.

* Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h:
(WebKit::NetworkConnectionToWebProcess::mdnsRegister):

* Source/WebKit/NetworkProcess/webrtc/NetworkMDNSRegister.cpp:
(WebKit::PendingRegistrationRequest::PendingRegistrationRequest):
(WebKit::NetworkMDNSRegister::closeAndForgetService):
(WebKit::registerMDNSNameCallback):
(WebKit::NetworkMDNSRegister::registerMDNSName):
* Source/WebKit/NetworkProcess/webrtc/NetworkMDNSRegister.h:

Canonical link: https://commits.webkit.org/266420@main
  • Loading branch information
beidson committed Jul 31, 2023
1 parent 3cb928b commit e50b54c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 7 deletions.
5 changes: 4 additions & 1 deletion Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ class NetworkConnectionToWebProcess
void addAllowedFirstPartyForCookies(const RegistrableDomain&);
void useRedirectionForCurrentNavigation(WebCore::ResourceLoaderIdentifier, WebCore::ResourceResponse&&);

#if ENABLE(WEB_RTC)
NetworkMDNSRegister& mdnsRegister() { return m_mdnsRegister; }
#endif

private:
NetworkConnectionToWebProcess(NetworkProcess&, WebCore::ProcessIdentifier, PAL::SessionID, NetworkProcessConnectionParameters, IPC::Connection::Identifier);

Expand Down Expand Up @@ -312,7 +316,6 @@ class NetworkConnectionToWebProcess
NetworkRTCProvider& rtcProvider();
#endif
#if ENABLE(WEB_RTC)
NetworkMDNSRegister& mdnsRegister() { return m_mdnsRegister; }
void registerToRTCDataChannelProxy();
void unregisterToRTCDataChannelProxy();
#endif
Expand Down
27 changes: 21 additions & 6 deletions Source/WebKit/NetworkProcess/webrtc/NetworkMDNSRegister.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ void NetworkMDNSRegister::unregisterMDNSNames(WebCore::ScriptExecutionContextIde

struct PendingRegistrationRequest {
WTF_MAKE_STRUCT_FAST_ALLOCATED;
PendingRegistrationRequest(Ref<IPC::Connection>&& connection, MDNSRegisterIdentifier requestIdentifier, String&& name, PAL::SessionID sessionID)
PendingRegistrationRequest(Ref<NetworkConnectionToWebProcess>&& connection, MDNSRegisterIdentifier requestIdentifier, String&& name, PAL::SessionID sessionID)
: connection(WTFMove(connection))
, requestIdentifier(requestIdentifier)
, name(WTFMove(name))
, sessionID(sessionID)
{
}

Ref<IPC::Connection> connection;
Ref<NetworkConnectionToWebProcess> connection;
MDNSRegisterIdentifier requestIdentifier;
String name;
PAL::SessionID sessionID;
Expand All @@ -89,7 +89,16 @@ static HashMap<uintptr_t, std::unique_ptr<PendingRegistrationRequest>>& pendingR
return map;
}

static void registerMDNSNameCallback(DNSServiceRef, DNSRecordRef record, DNSServiceFlags, DNSServiceErrorType errorCode, void *context)
void NetworkMDNSRegister::closeAndForgetService(DNSServiceRef service)
{
DNSServiceRefDeallocate(service);

m_services.removeIf([service] (auto& iterator) {
return iterator.value == service;
});
}

static void registerMDNSNameCallback(DNSServiceRef service, DNSRecordRef record, DNSServiceFlags, DNSServiceErrorType errorCode, void *context)
{
auto request = pendingRegistrationRequests().take(reinterpret_cast<uintptr_t>(context));
if (!request)
Expand All @@ -98,10 +107,12 @@ static void registerMDNSNameCallback(DNSServiceRef, DNSRecordRef record, DNSServ
MDNS_RELEASE_LOG_IN_CALLBACK(request->sessionID, "registerMDNSNameCallback with error %d", errorCode);

if (errorCode) {
request->connection->send(Messages::WebMDNSRegister::FinishedRegisteringMDNSName { request->requestIdentifier, request->name, WebCore::MDNSRegisterError::DNSSD }, 0);
request->connection->mdnsRegister().closeAndForgetService(service);
request->connection->connection().send(Messages::WebMDNSRegister::FinishedRegisteringMDNSName { request->requestIdentifier, request->name, WebCore::MDNSRegisterError::DNSSD }, 0);

return;
}
request->connection->send(Messages::WebMDNSRegister::FinishedRegisteringMDNSName { request->requestIdentifier, request->name, { } }, 0);
request->connection->connection().send(Messages::WebMDNSRegister::FinishedRegisteringMDNSName { request->requestIdentifier, request->name, { } }, 0);
}

void NetworkMDNSRegister::registerMDNSName(MDNSRegisterIdentifier requestIdentifier, WebCore::ScriptExecutionContextIdentifier documentIdentifier, const String& ipAddress)
Expand Down Expand Up @@ -137,7 +148,7 @@ void NetworkMDNSRegister::registerMDNSName(MDNSRegisterIdentifier requestIdentif
return;
}

auto pendingRequest = makeUnique<PendingRegistrationRequest>(m_connection.connection(), requestIdentifier, WTFMove(name), sessionID());
auto pendingRequest = makeUnique<PendingRegistrationRequest>(m_connection, requestIdentifier, WTFMove(name), sessionID());
auto* record = &pendingRequest->record;
auto error = DNSServiceRegisterRecord(service,
record,
Expand All @@ -157,6 +168,10 @@ void NetworkMDNSRegister::registerMDNSName(MDNSRegisterIdentifier requestIdentif
reinterpret_cast<void*>(pendingRegistrationRequestCount));
if (error) {
MDNS_RELEASE_LOG("registerMDNSName DNSServiceRegisterRecord error %d", error);

DNSServiceRefDeallocate(service);
m_services.remove(documentIdentifier);

m_connection.connection().send(Messages::WebMDNSRegister::FinishedRegisteringMDNSName { requestIdentifier, pendingRequest->name, WebCore::MDNSRegisterError::DNSSD }, 0);
return;
}
Expand Down
4 changes: 4 additions & 0 deletions Source/WebKit/NetworkProcess/webrtc/NetworkMDNSRegister.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ class NetworkMDNSRegister {

void didReceiveMessage(IPC::Connection&, IPC::Decoder&);

#if ENABLE_MDNS
void closeAndForgetService(DNSServiceRef);
#endif

private:
void unregisterMDNSNames(WebCore::ScriptExecutionContextIdentifier);
void registerMDNSName(MDNSRegisterIdentifier, WebCore::ScriptExecutionContextIdentifier, const String& ipAddress);
Expand Down

0 comments on commit e50b54c

Please sign in to comment.