Make Darwin HostNameRegistrar safer to use. (#25983)

There are two main changes here:

1) When we shut down the Matter stack, we should remove all our advertisements
adn stop monitoring for network interface changes.  That's what the change to
ChipDnssdShutdown does.

2) It's theoretically possible for the block HostNameRegistrar passes to
nw_path_monitor_set_update_handler to run after the HostNameRegistrar is
destroyed.  Guard against that with a shared_ptr that keeps track of the
liveness state of the HostNameRegistrar.

The changes to the member initialization of HostNameRegistrar are just for
safety's sake, in case its destructor ever gets called without Init() having
been called first.  Right now that is not possible, but better to not rely on
that.

src/platform/Darwin/DnssdHostNameRegistrar.cpp

@@ -256,13 +256,30 @@ void GetInterfaceAddresses(uint32_t interfaceId, chip::Inet::IPAddressType addre
 }
 } // namespace
 
-void HostNameRegistrar::Init(const char * hostname, Inet::IPAddressType addressType, uint32_t interfaceId)
+HostNameRegistrar::~HostNameRegistrar()
+{
+ Unregister();
+ if (mLivenessTracker != nullptr)
+ {
+ *mLivenessTracker = false;
+ }
+}
+
+DNSServiceErrorType HostNameRegistrar::Init(const char * hostname, Inet::IPAddressType addressType, uint32_t interfaceId)
 {
  mHostname = hostname;
  mInterfaceId = interfaceId;
  mAddressType = addressType;
  mServiceRef = nullptr;
  mInterfaceMonitor = nullptr;
+
+ mLivenessTracker = std::make_shared<bool>(true);
+ if (mLivenessTracker == nullptr)
+ {
+ return kDNSServiceErr_NoMemory;
+ }
+
+ return kDNSServiceErr_NoError;
 }
 
 CHIP_ERROR HostNameRegistrar::Register()
@@ -303,7 +320,18 @@ CHIP_ERROR HostNameRegistrar::StartMonitorInterfaces(OnInterfaceChanges interfac
 
  nw_path_monitor_set_queue(mInterfaceMonitor, chip::DeviceLayer::PlatformMgrImpl().GetWorkQueue());
 
+ // Our update handler closes over "this", but can't keep us alive (because we
+ // are not refcounted). Make sure it closes over a shared ref to our
+ // liveness tracker, which it _can_ keep alive, so it can bail out if we
+ // have been destroyed between when the task was queued and when it ran.
+ std::shared_ptr<bool> livenessTracker = mLivenessTracker;
  nw_path_monitor_set_update_handler(mInterfaceMonitor, ^(nw_path_t path) {
+ if (!*livenessTracker)
+ {
+ // The HostNameRegistrar has been destroyed; just bail out.
+ return;
+ }
+
 #if CHIP_PROGRESS_LOGGING
  LogDetails(path);
 #endif // CHIP_PROGRESS_LOGGING

src/platform/Darwin/DnssdHostNameRegistrar.h

@@ -36,7 +36,9 @@ namespace Dnssd {
 
  class HostNameRegistrar {
  public:
- void Init(const char * hostname, Inet::IPAddressType addressType, uint32_t interfaceId);
+ ~HostNameRegistrar();
+
+ DNSServiceErrorType Init(const char * hostname, Inet::IPAddressType addressType, uint32_t interfaceId);
 
  CHIP_ERROR Register();
  void Unregister();
@@ -61,15 +63,24 @@ namespace Dnssd {
  CHIP_ERROR StartSharedConnection();
  void StopSharedConnection();
  CHIP_ERROR ResetSharedConnection();
- DNSServiceRef mServiceRef;
 
  CHIP_ERROR StartMonitorInterfaces(OnInterfaceChanges interfaceChangesBlock);
  void StopMonitorInterfaces();
- nw_path_monitor_t mInterfaceMonitor;
+
+ DNSServiceRef mServiceRef = nullptr;
+ nw_path_monitor_t mInterfaceMonitor = nullptr;
 
  std::string mHostname;
- uint32_t mInterfaceId;
+ // Default to kDNSServiceInterfaceIndexLocalOnly so we don't mess around
+ // with un-registration if we never get Init() called.
+ uint32_t mInterfaceId = kDNSServiceInterfaceIndexLocalOnly;
  Inet::IPAddressType mAddressType;
+
+ // We use mLivenessTracker to indicate to blocks that close over us that
+ // we've been destroyed. This is needed because we're not a refcounted
+ // object, so the blocks can't keep us alive; they just close over the
+ // raw pointer to "this".
+ std::shared_ptr<bool> mLivenessTracker;
  };
 
 } // namespace Dnssd

src/platform/Darwin/DnssdImpl.cpp

@@ -195,11 +195,12 @@ CHIP_ERROR Register(void * context, DnssdPublishCallback callback, uint32_t inte
  sdCtx = chip::Platform::New<RegisterContext>(type, name, callback, context);
  VerifyOrReturnError(nullptr != sdCtx, CHIP_ERROR_NO_MEMORY);
 
- sdCtx->mHostNameRegistrar.Init(hostname, addressType, interfaceId);
+ auto err = sdCtx->mHostNameRegistrar.Init(hostname, addressType, interfaceId);
+ VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
 
  DNSServiceRef sdRef;
- auto err = DNSServiceRegister(&sdRef, kRegisterFlags, interfaceId, name, type, kLocalDot, hostname, htons(port), record.size(),
-  record.data(), OnRegister, sdCtx);
+ err = DNSServiceRegister(&sdRef, kRegisterFlags, interfaceId, name, type, kLocalDot, hostname, htons(port), record.size(),
+ record.data(), OnRegister, sdCtx);
  VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
 
  return MdnsContexts::GetInstance().Add(sdCtx, sdRef);
@@ -394,7 +395,12 @@ CHIP_ERROR ChipDnssdInit(DnssdAsyncReturnCallback successCallback, DnssdAsyncRet
  return CHIP_NO_ERROR;
 }
 
-void ChipDnssdShutdown() {}
+void ChipDnssdShutdown()
+{
+ // Drop our existing advertisements now, so they don't stick around while we
+ // are not actually in a responsive state.
+ ChipDnssdRemoveServices();
+}
 
 CHIP_ERROR ChipDnssdPublishService(const DnssdService * service, DnssdPublishCallback callback, void * context)
 {
@@ -416,6 +422,8 @@ CHIP_ERROR ChipDnssdPublishService(const DnssdService * service, DnssdPublishCal
 
 CHIP_ERROR ChipDnssdRemoveServices()
 {
+ assertChipStackLockedByCurrentThread();
+
  auto err = MdnsContexts::GetInstance().RemoveAllOfType(ContextType::Register);
  if (CHIP_ERROR_KEY_NOT_FOUND == err)
  {

src/platform/Darwin/MdnsError.cpp

@@ -102,6 +102,8 @@ CHIP_ERROR ToChipError(DNSServiceErrorType errorCode)
  return CHIP_NO_ERROR;
  case kDNSServiceErr_NameConflict:
  return CHIP_ERROR_MDNS_COLLISION;
+ case kDNSServiceErr_NoMemory:
+ return CHIP_ERROR_NO_MEMORY;
  default:
  return CHIP_ERROR_INTERNAL;
  }