Add a ResolveProxy class to keep track of which controller is issuing… (

project-chip#12481)

* Add a ResolveProxy class to keep track of which controller is issuing a dns request

* Use ReferenceCount<ResolverDelegateProxy> to make sure the stack is not trying to access a dangling pointer once a response is received

src/app/CASESessionManager.cpp

@@ -53,7 +53,7 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(NodeId nodeId, Callback::C
         session->OnNodeIdResolved(resolutionData);
     }
 
-    CHIP_ERROR err = session->Connect(onConnection, onFailure);
+    CHIP_ERROR err = session->Connect(onConnection, onFailure, mConfig.dnsResolver);
     if (err != CHIP_NO_ERROR)
     {
         ReleaseSession(session);
@@ -69,8 +69,8 @@ void CASESessionManager::ReleaseSession(NodeId nodeId)
 
 CHIP_ERROR CASESessionManager::ResolveDeviceAddress(NodeId nodeId)
 {
-    return Dnssd::Resolver::Instance().ResolveNodeId(GetFabricInfo()->GetPeerIdForNode(nodeId), Inet::IPAddressType::kAny,
-                                                     Dnssd::Resolver::CacheBypass::On);
+    return mConfig.dnsResolver->ResolveNodeId(GetFabricInfo()->GetPeerIdForNode(nodeId), Inet::IPAddressType::kAny,
+                                              Dnssd::Resolver::CacheBypass::On);
 }
 
 void CASESessionManager::OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData)

src/app/CASESessionManager.h

@@ -27,7 +27,7 @@
 #include <lib/support/Pool.h>
 #include <transport/SessionDelegate.h>
 
-#include <lib/dnssd/Resolver.h>
+#include <lib/dnssd/ResolverProxy.h>
 
 namespace chip {
 
@@ -36,6 +36,7 @@ struct CASESessionManagerConfig
     DeviceProxyInitParams sessionInitParams;
     Dnssd::DnssdCache<CHIP_CONFIG_MDNS_CACHE_SIZE> * dnsCache = nullptr;
     OperationalDeviceProxyPoolDelegate * devicePool           = nullptr;
+    Dnssd::ResolverProxy * dnsResolver                        = nullptr;
 };
 
 /**

src/app/OperationalDeviceProxy.cpp

@@ -43,7 +43,8 @@ using namespace chip::Callback;
 namespace chip {
 
 CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback<OnDeviceConnected> * onConnection,
-                                           Callback::Callback<OnDeviceConnectionFailure> * onFailure)
+                                           Callback::Callback<OnDeviceConnectionFailure> * onFailure,
+                                           Dnssd::ResolverProxy * resolver)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
 
@@ -54,7 +55,8 @@ CHIP_ERROR OperationalDeviceProxy::Connect(Callback::Callback<OnDeviceConnected>
         break;
 
     case State::NeedsAddress:
-        err = Dnssd::Resolver::Instance().ResolveNodeId(mPeerId, chip::Inet::IPAddressType::kAny);
+        VerifyOrReturnError(resolver != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
+        err = resolver->ResolveNodeId(mPeerId, chip::Inet::IPAddressType::kAny);
         EnqueueConnectionCallbacks(onConnection, onFailure);
         break;
 

src/app/OperationalDeviceProxy.h

@@ -43,7 +43,7 @@
 #include <transport/raw/MessageHeader.h>
 #include <transport/raw/UDP.h>
 
-#include <lib/dnssd/Resolver.h>
+#include <lib/dnssd/ResolverProxy.h>
 
 namespace chip {
 
@@ -109,9 +109,11 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
      * session setup fails, `onFailure` will be called.
      *
      * If the session already exists, `onConnection` will be called immediately.
+     * If the resolver is null and the device state is State::NeedsAddress, CHIP_ERROR_INVALID_ARGUMENT will be
+     * returned.
      */
     CHIP_ERROR Connect(Callback::Callback<OnDeviceConnected> * onConnection,
-                       Callback::Callback<OnDeviceConnectionFailure> * onFailure);
+                       Callback::Callback<OnDeviceConnectionFailure> * onFailure, Dnssd::ResolverProxy * resolver);
 
     bool IsConnected() const { return mState == State::SecureConnected; }
 

src/app/clusters/ota-requestor/OTARequestor.cpp

@@ -235,6 +235,7 @@ CHIP_ERROR OTARequestor::SetupCASESessionManager(FabricIndex fabricIndex)
             .sessionInitParams = initParams,
             .dnsCache          = nullptr,
             .devicePool        = mServer->GetDevicePool(),
+            .dnsResolver       = nullptr,
         };
 
         mCASESessionManager = Platform::New<CASESessionManager>(sessionManagerConfig);

src/controller/AbstractDnssdDiscoveryController.cpp

@@ -19,10 +19,6 @@
 // module header, comes first
 #include <controller/AbstractDnssdDiscoveryController.h>
 
-#if CONFIG_DEVICE_LAYER
-#include <platform/CHIPDeviceLayer.h>
-#endif
-
 #include <lib/core/CHIPEncoding.h>
 #include <lib/support/logging/CHIPLogging.h>
 
@@ -66,11 +62,6 @@ void AbstractDnssdDiscoveryController::OnNodeDiscoveryComplete(const chip::Dnssd
 
 CHIP_ERROR AbstractDnssdDiscoveryController::SetUpNodeDiscovery()
 {
-#if CONFIG_DEVICE_LAYER
-    ReturnErrorOnFailure(mResolver->Init(&DeviceLayer::InetLayer()));
-#endif
-    mResolver->SetResolverDelegate(this);
-
     auto discoveredNodes = GetDiscoveredNodes();
     for (auto & discoveredNode : discoveredNodes)
     {

src/controller/AbstractDnssdDiscoveryController.h

@@ -19,7 +19,7 @@
 #pragma once
 
 #include <controller/DeviceDiscoveryDelegate.h>
-#include <lib/dnssd/Resolver.h>
+#include <lib/dnssd/ResolverProxy.h>
 #include <lib/support/Span.h>
 #include <platform/CHIPDeviceConfig.h>
 
@@ -39,11 +39,7 @@ namespace Controller {
 class DLL_EXPORT AbstractDnssdDiscoveryController : public Dnssd::ResolverDelegate
 {
 public:
-    AbstractDnssdDiscoveryController(chip::Dnssd::Resolver * resolver = &chip::Dnssd::Resolver::Instance())
-    {
-        mResolver = resolver;
-    }
-
+    AbstractDnssdDiscoveryController() {}
     virtual ~AbstractDnssdDiscoveryController() {}
 
     void OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
@@ -52,9 +48,9 @@ class DLL_EXPORT AbstractDnssdDiscoveryController : public Dnssd::ResolverDelega
     using DiscoveredNodeList = FixedSpan<Dnssd::DiscoveredNodeData, CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES>;
     CHIP_ERROR SetUpNodeDiscovery();
     const Dnssd::DiscoveredNodeData * GetDiscoveredNode(int idx);
-    virtual DiscoveredNodeList GetDiscoveredNodes() = 0;
-    chip::Dnssd::Resolver * mResolver;
+    virtual DiscoveredNodeList GetDiscoveredNodes()    = 0;
     DeviceDiscoveryDelegate * mDeviceDiscoveryDelegate = nullptr;
+    Dnssd::ResolverProxy mDNSResolver;
 };
 
 } // namespace Controller

src/controller/CHIPCommissionableNodeController.cpp

@@ -19,6 +19,10 @@
 // module header, comes first
 #include <controller/CHIPCommissionableNodeController.h>
 
+#if CONFIG_DEVICE_LAYER
+#include <platform/CHIPDeviceLayer.h>
+#endif
+
 #include <lib/support/CodeUtils.h>
 
 namespace chip {
@@ -27,7 +31,22 @@ namespace Controller {
 CHIP_ERROR CommissionableNodeController::DiscoverCommissioners(Dnssd::DiscoveryFilter discoveryFilter)
 {
     ReturnErrorOnFailure(SetUpNodeDiscovery());
-    return mResolver->FindCommissioners(discoveryFilter);
+
+    if (mResolver == nullptr)
+    {
+#if CONFIG_DEVICE_LAYER
+        ReturnErrorOnFailure(mDNSResolver.Init(&DeviceLayer::InetLayer()));
+#endif
+        mDNSResolver.SetResolverDelegate(this);
+        return mDNSResolver.FindCommissioners(discoveryFilter);
+    }
+    else
+    {
+#if CONFIG_DEVICE_LAYER
+        ReturnErrorOnFailure(mResolver->Init(&DeviceLayer::InetLayer()));
+#endif
+        return mResolver->FindCommissioners(discoveryFilter);
+    }
 }
 
 const Dnssd::DiscoveredNodeData * CommissionableNodeController::GetDiscoveredCommissioner(int idx)

src/controller/CHIPCommissionableNodeController.h

@@ -19,7 +19,6 @@
 #pragma once
 
 #include <controller/AbstractDnssdDiscoveryController.h>
-#include <lib/dnssd/Resolver.h>
 #include <lib/support/logging/CHIPLogging.h>
 #include <platform/CHIPDeviceConfig.h>
 
@@ -37,8 +36,7 @@ namespace Controller {
 class DLL_EXPORT CommissionableNodeController : public AbstractDnssdDiscoveryController
 {
 public:
-    CommissionableNodeController(chip::Dnssd::Resolver * resolver = &chip::Dnssd::Resolver::Instance()) :
-        AbstractDnssdDiscoveryController(resolver){};
+    CommissionableNodeController(chip::Dnssd::Resolver * resolver = nullptr) : mResolver(resolver) {}
     virtual ~CommissionableNodeController() {}
 
     CHIP_ERROR DiscoverCommissioners(Dnssd::DiscoveryFilter discoveryFilter = Dnssd::DiscoveryFilter());
@@ -66,6 +64,7 @@ class DLL_EXPORT CommissionableNodeController : public AbstractDnssdDiscoveryCon
     DiscoveredNodeList GetDiscoveredNodes() override { return DiscoveredNodeList(mDiscoveredCommissioners); }
 
 private:
+    Dnssd::Resolver * mResolver = nullptr;
     Dnssd::DiscoveredNodeData mDiscoveredCommissioners[CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES];
 };
 

src/controller/CHIPDeviceController.cpp

@@ -45,7 +45,6 @@
 
 #include <app/InteractionModelEngine.h>
 #include <app/OperationalDeviceProxy.h>
-#include <app/util/DataModelHandler.h>
 #include <app/util/error-mapping.h>
 #include <credentials/CHIPCert.h>
 #include <credentials/DeviceAttestationCredsProvider.h>
@@ -132,14 +131,12 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params)
     VerifyOrReturnError(params.systemState->TransportMgr() != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-    Dnssd::Resolver::Instance().Init(params.systemState->InetLayer());
-    Dnssd::Resolver::Instance().SetResolverDelegate(this);
+    ReturnErrorOnFailure(mDNSResolver.Init(params.systemState->InetLayer()));
+    mDNSResolver.SetResolverDelegate(this);
     RegisterDeviceAddressUpdateDelegate(params.deviceAddressUpdateDelegate);
     RegisterDeviceDiscoveryDelegate(params.deviceDiscoveryDelegate);
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
 
-    InitDataModelHandler(params.systemState->ExchangeMgr());
-
     VerifyOrReturnError(params.operationalCredentialsDelegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     mOperationalCredentialsDelegate = params.operationalCredentialsDelegate;
 
@@ -160,6 +157,11 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params)
         .sessionInitParams = deviceInitParams,
         .dnsCache          = &mDNSCache,
         .devicePool        = &mDevicePool,
+#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
+        .dnsResolver = &mDNSResolver,
+#else
+        .dnsResolver = nullptr,
+#endif
     };
 
     mCASESessionManager = chip::Platform::New<CASESessionManager>(sessionManagerConfig);
@@ -227,18 +229,14 @@ CHIP_ERROR DeviceController::Shutdown()
 
     mState = State::NotInitialized;
 
-#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-    Dnssd::Resolver::Instance().Shutdown();
-#endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-
     mStorageDelegate = nullptr;
 
     mSystemState->Fabrics()->ReleaseFabricIndex(mFabricIndex);
     mSystemState->Release();
     mSystemState = nullptr;
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-    Dnssd::Resolver::Instance().SetResolverDelegate(nullptr);
+    mDNSResolver.Shutdown();
     mDeviceAddressUpdateDelegate = nullptr;
     mDeviceDiscoveryDelegate     = nullptr;
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
@@ -1572,7 +1570,7 @@ void DeviceCommissioner::OnSessionEstablishmentTimeoutCallback(System::Layer * a
 CHIP_ERROR DeviceCommissioner::DiscoverCommissionableNodes(Dnssd::DiscoveryFilter filter)
 {
     ReturnErrorOnFailure(SetUpNodeDiscovery());
-    return chip::Dnssd::Resolver::Instance().FindCommissionableNodes(filter);
+    return mDNSResolver.FindCommissionableNodes(filter);
 }
 
 const Dnssd::DiscoveredNodeData * DeviceCommissioner::GetDiscoveredDevice(int idx)
@@ -1838,7 +1836,7 @@ void DeviceCommissioner::AdvanceCommissioningStage(CHIP_ERROR err)
 #if CONFIG_DEVICE_LAYER
         status = DeviceLayer::ConfigurationMgr().GetCountryCode(countryCodeStr, kMaxCountryCodeSize, actualCountryCodeSize);
 #else
-        status            = CHIP_ERROR_NOT_IMPLEMENTED;
+        status = CHIP_ERROR_NOT_IMPLEMENTED;
 #endif
         if (status != CHIP_NO_ERROR)
         {
@@ -1895,7 +1893,7 @@ void DeviceCommissioner::AdvanceCommissioningStage(CHIP_ERROR err)
         RendezvousCleanup(CHIP_NO_ERROR);
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
         ChipLogProgress(Controller, "Finding node on operational network");
-        Dnssd::Resolver::Instance().ResolveNodeId(peerId, Inet::IPAddressType::kAny);
+        mDNSResolver.ResolveNodeId(peerId, Inet::IPAddressType::kAny);
 #endif
     }
     break;

src/controller/CHIPDeviceController.h

@@ -72,6 +72,7 @@
 #include <controller/DeviceAddressUpdateDelegate.h>
 #include <controller/DeviceDiscoveryDelegate.h>
 #include <lib/dnssd/Resolver.h>
+#include <lib/dnssd/ResolverProxy.h>
 #endif
 
 namespace chip {

src/controller/CHIPDeviceControllerFactory.cpp

@@ -24,6 +24,7 @@
 
 #include <controller/CHIPDeviceControllerFactory.h>
 
+#include <app/util/DataModelHandler.h>
 #include <lib/support/ErrorStr.h>
 
 #if CONFIG_DEVICE_LAYER
@@ -140,9 +141,15 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
     ReturnErrorOnFailure(stateParams.exchangeMgr->Init(stateParams.sessionMgr));
     ReturnErrorOnFailure(stateParams.messageCounterManager->Init(stateParams.exchangeMgr));
 
+    InitDataModelHandler(stateParams.exchangeMgr);
+
     stateParams.imDelegate = params.imDelegate;
     ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->Init(stateParams.exchangeMgr, stateParams.imDelegate));
 
+#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
+    ReturnErrorOnFailure(Dnssd::Resolver::Instance().Init(stateParams.inetLayer));
+#endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
+
     // store the system state
     mSystemState = chip::Platform::New<DeviceControllerSystemState>(stateParams);
     ChipLogDetail(Controller, "System State Initialized...");
@@ -220,6 +227,10 @@ CHIP_ERROR DeviceControllerSystemState::Shutdown()
 
     ChipLogDetail(Controller, "Shutting down the System State, this will teardown the CHIP Stack");
 
+#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
+    Dnssd::Resolver::Instance().Shutdown();
+#endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
+
     // Shut down the interaction model
     app::InteractionModelEngine::GetInstance()->Shutdown();
 

src/controller/tests/TestCommissionableNodeController.cpp

@@ -180,9 +180,26 @@ const nlTest sTests[] =
 
 } // namespace
 
+int TestCommissionableNodeController_Setup(void * inContext)
+{
+    if (CHIP_NO_ERROR != chip::Platform::MemoryInit())
+    {
+        return FAILURE;
+    }
+
+    return SUCCESS;
+}
+
+int TestCommissionableNodeController_Teardown(void * inContext)
+{
+    chip::Platform::MemoryShutdown();
+    return SUCCESS;
+}
+
 int TestCommissionableNodeController()
 {
-    nlTestSuite theSuite = { "CommissionableNodeController", &sTests[0], NULL, NULL };
+    nlTestSuite theSuite = { "CommissionableNodeController", &sTests[0], TestCommissionableNodeController_Setup,
+                             TestCommissionableNodeController_Teardown };
     nlTestRunner(&theSuite, nullptr);
     return nlTestRunnerStats(&theSuite);
 }