[wpinet] Switch macOS mDNSResolver to dispatch_queue (#8621)

The old thread based method had issues and occasionally segfaulted. And
was much more complex. This is much cleaner
This commit is contained in:
Thad House
2026-02-17 20:20:34 -08:00
committed by GitHub
parent af01a72725
commit 10fa2fced4
3 changed files with 60 additions and 215 deletions

View File

@@ -16,7 +16,6 @@
#include <utility>
#include <vector>
#include "ResolverThread.hpp"
#include "dns_sd.h"
#include "wpi/util/SmallVector.hpp"
@@ -38,7 +37,7 @@ struct DnsResolveState {
struct MulticastServiceResolver::Impl {
std::string serviceType;
MulticastServiceResolver* resolver;
std::shared_ptr<ResolverThread> thread = ResolverThread::Get();
dispatch_queue_t queue;
std::vector<std::unique_ptr<DnsResolveState>> ResolveStates;
DNSServiceRef serviceRef = nullptr;
@@ -64,20 +63,16 @@ void ServiceGetAddrInfoReply(DNSServiceRef sdRef, DNSServiceFlags flags,
const char* hostname,
const struct sockaddr* address, uint32_t ttl,
void* context) {
if (errorCode != kDNSServiceErr_NoError) {
return;
DnsResolveState* resolveState = static_cast<DnsResolveState*>(context);
if (errorCode == kDNSServiceErr_NoError) {
resolveState->data.hostName = hostname;
resolveState->data.ipv4Address = ntohl(
reinterpret_cast<const struct sockaddr_in*>(address)->sin_addr.s_addr);
resolveState->pImpl->onFound(std::move(resolveState->data));
}
DnsResolveState* resolveState = static_cast<DnsResolveState*>(context);
resolveState->data.hostName = hostname;
resolveState->data.ipv4Address = ntohl(
reinterpret_cast<const struct sockaddr_in*>(address)->sin_addr.s_addr);
resolveState->pImpl->onFound(std::move(resolveState->data));
resolveState->pImpl->thread->RemoveServiceRefInThread(
resolveState->ResolveRef);
DNSServiceRefDeallocate(resolveState->ResolveRef);
resolveState->pImpl->ResolveStates.erase(std::find_if(
resolveState->pImpl->ResolveStates.begin(),
@@ -91,15 +86,16 @@ void ServiceResolveReply(DNSServiceRef sdRef, DNSServiceFlags flags,
uint16_t port, /* In network byte order */
uint16_t txtLen, const unsigned char* txtRecord,
void* context) {
if (errorCode != kDNSServiceErr_NoError) {
return;
}
DnsResolveState* resolveState = static_cast<DnsResolveState*>(context);
resolveState->pImpl->thread->RemoveServiceRefInThread(
resolveState->ResolveRef);
DNSServiceRefDeallocate(resolveState->ResolveRef);
resolveState->ResolveRef = nullptr;
if (errorCode != kDNSServiceErr_NoError) {
resolveState->pImpl->ResolveStates.erase(std::find_if(
resolveState->pImpl->ResolveStates.begin(),
resolveState->pImpl->ResolveStates.end(),
[resolveState](auto& a) { return a.get() == resolveState; }));
}
resolveState->data.port = ntohs(port);
int txtCount = TXTRecordGetCount(txtLen, txtRecord);
@@ -128,12 +124,16 @@ void ServiceResolveReply(DNSServiceRef sdRef, DNSServiceFlags flags,
kDNSServiceProtocol_IPv4, hosttarget, ServiceGetAddrInfoReply, context);
if (errorCode == kDNSServiceErr_NoError) {
dnssd_sock_t socket = DNSServiceRefSockFD(resolveState->ResolveRef);
resolveState->pImpl->thread->AddServiceRef(resolveState->ResolveRef,
socket);
errorCode = DNSServiceSetDispatchQueue(resolveState->ResolveRef,
resolveState->pImpl->queue);
} else {
resolveState->pImpl->thread->RemoveServiceRefInThread(
resolveState->ResolveRef);
resolveState->ResolveRef = nullptr;
}
if (errorCode != kDNSServiceErr_NoError) {
if (resolveState->ResolveRef) {
DNSServiceRefDeallocate(resolveState->ResolveRef);
}
resolveState->pImpl->ResolveStates.erase(std::find_if(
resolveState->pImpl->ResolveStates.begin(),
resolveState->pImpl->ResolveStates.end(),
@@ -163,10 +163,16 @@ static void DnsCompletion(DNSServiceRef sdRef, DNSServiceFlags flags,
ServiceResolveReply, resolveState.get());
if (errorCode == kDNSServiceErr_NoError) {
dnssd_sock_t socket = DNSServiceRefSockFD(resolveState->ResolveRef);
resolveState->pImpl->thread->AddServiceRef(resolveState->ResolveRef,
socket);
errorCode =
DNSServiceSetDispatchQueue(resolveState->ResolveRef, impl->queue);
} else {
resolveState->ResolveRef = nullptr;
}
if (errorCode != kDNSServiceErr_NoError) {
if (resolveState->ResolveRef) {
DNSServiceRefDeallocate(resolveState->ResolveRef);
}
resolveState->pImpl->ResolveStates.erase(std::find_if(
resolveState->pImpl->ResolveStates.begin(),
resolveState->pImpl->ResolveStates.end(),
@@ -201,12 +207,19 @@ void MulticastServiceResolver::Start() {
return;
}
pImpl->queue = dispatch_queue_create(nullptr, DISPATCH_QUEUE_SERIAL);
DNSServiceErrorType status =
DNSServiceBrowse(&pImpl->serviceRef, 0, 0, pImpl->serviceType.c_str(),
"local", DnsCompletion, pImpl.get());
if (status == kDNSServiceErr_NoError) {
dnssd_sock_t socket = DNSServiceRefSockFD(pImpl->serviceRef);
pImpl->thread->AddServiceRef(pImpl->serviceRef, socket);
status = DNSServiceSetDispatchQueue(pImpl->serviceRef, pImpl->queue);
if (status != kDNSServiceErr_NoError) {
DNSServiceRefDeallocate(pImpl->serviceRef);
pImpl->serviceRef = nullptr;
dispatch_release(pImpl->queue);
pImpl->queue = nullptr;
}
}
}
@@ -214,25 +227,24 @@ void MulticastServiceResolver::Stop() {
if (!pImpl->serviceRef) {
return;
}
wpi::util::SmallVector<WPI_EventHandle, 8> cleanupEvents;
for (auto&& i : pImpl->ResolveStates) {
cleanupEvents.push_back(
pImpl->thread->RemoveServiceRefOutsideThread(i->ResolveRef));
}
cleanupEvents.push_back(
pImpl->thread->RemoveServiceRefOutsideThread(pImpl->serviceRef));
wpi::util::SmallVector<WPI_Handle, 8> signaledBuf;
signaledBuf.resize(cleanupEvents.size());
while (!cleanupEvents.empty()) {
auto signaled = wpi::util::WaitForObjects(cleanupEvents, signaledBuf);
for (auto&& s : signaled) {
cleanupEvents.erase(
std::find(cleanupEvents.begin(), cleanupEvents.end(), s));
}
}
pImpl->ResolveStates.clear();
pImpl->serviceRef = nullptr;
dispatch_sync_f(pImpl->queue, pImpl.get(), [](void* context) {
MulticastServiceResolver::Impl* impl =
static_cast<MulticastServiceResolver::Impl*>(context);
DNSServiceRefDeallocate(impl->serviceRef);
impl->serviceRef = nullptr;
for (auto&& i : impl->ResolveStates) {
if (i->ResolveRef) {
DNSServiceRefDeallocate(i->ResolveRef);
i->ResolveRef = nullptr;
}
}
impl->ResolveStates.clear();
});
dispatch_release(pImpl->queue);
pImpl->queue = nullptr;
}
#endif // defined(__APPLE__)

View File

@@ -1,118 +0,0 @@
// Copyright (c) FIRST and other WPILib contributors.
// Open Source Software; you can modify and/or share it under the terms of
// the WPILib BSD license file in the root directory of this project.
#if defined(__APPLE__)
#include "ResolverThread.hpp"
#include <algorithm>
#include <memory>
#include <utility>
#include <vector>
#include "wpi/util/mutex.hpp"
using namespace wpi::net;
ResolverThread::ResolverThread(const private_init&) {}
ResolverThread::~ResolverThread() noexcept {
running = false;
if (thread.joinable()) {
thread.join();
}
}
void ResolverThread::AddServiceRef(DNSServiceRef serviceRef,
dnssd_sock_t socket) {
std::scoped_lock lock{serviceRefMutex};
serviceRefs.emplace_back(
std::pair<DNSServiceRef, dnssd_sock_t>{serviceRef, socket});
if (serviceRefs.size() == 1) {
running = false;
if (thread.joinable()) {
thread.join();
}
running = true;
thread = std::thread([this] { ThreadMain(); });
}
}
void ResolverThread::RemoveServiceRefInThread(DNSServiceRef serviceRef) {
std::scoped_lock lock{serviceRefMutex};
serviceRefs.erase(
std::find_if(serviceRefs.begin(), serviceRefs.end(),
[=](auto& a) { return a.first == serviceRef; }));
DNSServiceRefDeallocate(serviceRef);
}
WPI_EventHandle ResolverThread::RemoveServiceRefOutsideThread(
DNSServiceRef serviceRef) {
std::scoped_lock lock{serviceRefMutex};
WPI_EventHandle handle = wpi::util::CreateEvent(true);
serviceRefsToRemove.push_back({serviceRef, handle});
return handle;
}
bool ResolverThread::CleanupRefs() {
std::scoped_lock lock{serviceRefMutex};
for (auto&& r : serviceRefsToRemove) {
serviceRefs.erase(
std::find_if(serviceRefs.begin(), serviceRefs.end(),
[=](auto& a) { return a.first == r.first; }));
DNSServiceRefDeallocate(r.first);
wpi::util::SetEvent(r.second);
}
serviceRefsToRemove.clear();
return serviceRefs.empty();
}
void ResolverThread::ThreadMain() {
std::vector<pollfd> readSockets;
std::vector<DNSServiceRef> serviceRefs;
while (running) {
readSockets.clear();
serviceRefs.clear();
for (auto&& i : this->serviceRefs) {
readSockets.emplace_back(pollfd{i.second, POLLIN, 0});
serviceRefs.emplace_back(i.first);
}
int res = poll(readSockets.begin().base(), readSockets.size(), 100);
if (res > 0) {
for (size_t i = 0; i < readSockets.size(); i++) {
if (readSockets[i].revents == POLLIN) {
DNSServiceProcessResult(serviceRefs[i]);
}
}
} else if (res == 0) {
if (!running) {
CleanupRefs();
break;
}
}
if (CleanupRefs()) {
break;
}
}
}
static wpi::util::mutex ThreadLoopLock;
static std::weak_ptr<ResolverThread> ThreadLoop;
std::shared_ptr<ResolverThread> ResolverThread::Get() {
std::scoped_lock lock{ThreadLoopLock};
auto locked = ThreadLoop.lock();
if (!locked) {
locked = std::make_unique<ResolverThread>(private_init{});
ThreadLoop = locked;
}
return locked;
}
#endif // defined(__APPLE__)

View File

@@ -1,49 +0,0 @@
// Copyright (c) FIRST and other WPILib contributors.
// Open Source Software; you can modify and/or share it under the terms of
// the WPILib BSD license file in the root directory of this project.
#if defined(__APPLE__)
#pragma once
#include <netinet/in.h>
#include <poll.h>
#include <atomic>
#include <memory>
#include <thread>
#include <utility>
#include <vector>
#include "dns_sd.h"
#include "wpi/util/Synchronization.h"
#include "wpi/util/mutex.hpp"
namespace wpi::net {
class ResolverThread {
private:
struct private_init {};
public:
explicit ResolverThread(const private_init&);
~ResolverThread() noexcept;
void AddServiceRef(DNSServiceRef serviceRef, dnssd_sock_t socket);
void RemoveServiceRefInThread(DNSServiceRef serviceRef);
WPI_EventHandle RemoveServiceRefOutsideThread(DNSServiceRef serviceRef);
static std::shared_ptr<ResolverThread> Get();
private:
void ThreadMain();
bool CleanupRefs();
wpi::util::mutex serviceRefMutex;
std::vector<std::pair<DNSServiceRef, WPI_EventHandle>> serviceRefsToRemove;
std::vector<std::pair<DNSServiceRef, dnssd_sock_t>> serviceRefs;
std::thread thread;
std::atomic_bool running;
};
} // namespace wpi::net
#endif // defined(__APPLE__)