openthread · Irving-cl · Nov 25, 2024
diff --git a/src/dbus/server/dbus_thread_object_ncp.cpp b/src/dbus/server/dbus_thread_object_ncp.cpp
@@ -125,7 +125,7 @@ void DBusThreadObjectNcp::JoinHandler(DBusRequest &aRequest)
 
 void DBusThreadObjectNcp::LeaveHandler(DBusRequest &aRequest)
 {
-    mHost.Leave([aRequest](otError aError, const std::string &aErrorInfo) mutable {
+    mHost.Leave(true /* aEraseDataset */, [aRequest](otError aError, const std::string &aErrorInfo) mutable {
         OT_UNUSED_VARIABLE(aErrorInfo);
         aRequest.ReplyOtResult(aError);
     });

diff --git a/src/ncp/ncp_host.cpp b/src/ncp/ncp_host.cpp
@@ -157,14 +157,23 @@ void NcpHost::Join(const otOperationalDatasetTlvs &aActiveOpDatasetTlvs, const A
     task->Run();
 }
 
-void NcpHost::Leave(const AsyncResultReceiver &aReceiver)
+void NcpHost::Leave(bool aEraseDataset, const AsyncResultReceiver &aReceiver)
 {
     AsyncTaskPtr task;
     auto errorHandler = [aReceiver](otError aError, const std::string &aErrorInfo) { aReceiver(aError, aErrorInfo); };
 
     task = std::make_shared<AsyncTask>(errorHandler);
     task->First([this](AsyncTaskPtr aNext) { mNcpSpinel.ThreadDetachGracefully(std::move(aNext)); })
-        ->Then([this](AsyncTaskPtr aNext) { mNcpSpinel.ThreadErasePersistentInfo(std::move(aNext)); });
+        ->Then([this, aEraseDataset](AsyncTaskPtr aNext) {
+            if (aEraseDataset)
+            {
+                mNcpSpinel.ThreadErasePersistentInfo(std::move(aNext));
+            }
+            else
+            {
+                aNext->SetResult(OT_ERROR_NONE, "");
+            }
+        });
     task->Run();
 }
 

diff --git a/src/ncp/ncp_host.hpp b/src/ncp/ncp_host.hpp
@@ -91,7 +91,7 @@ class NcpHost : public MainloopProcessor, public ThreadHost, public NcpNetworkPr
 
     // ThreadHost methods
     void Join(const otOperationalDatasetTlvs &aActiveOpDatasetTlvs, const AsyncResultReceiver &aReceiver) override;
-    void Leave(const AsyncResultReceiver &aReceiver) override;
+    void Leave(bool aEraseDataset, const AsyncResultReceiver &aReceiver) override;
     void ScheduleMigration(const otOperationalDatasetTlvs &aPendingOpDatasetTlvs,
                            const AsyncResultReceiver       aReceiver) override;
     void SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceiver) override;

diff --git a/src/ncp/rcp_host.cpp b/src/ncp/rcp_host.cpp
@@ -333,6 +333,7 @@ void RcpHost::Deinit(void)
 
     mSetThreadEnabledReceiver  = nullptr;
     mScheduleMigrationReceiver = nullptr;
+    mDetachGracefullyCallbacks.clear();
 }
 
 void RcpHost::HandleStateChanged(otChangedFlags aFlags)
@@ -445,10 +446,35 @@ void RcpHost::Join(const otOperationalDatasetTlvs &aActiveOpDatasetTlvs, const A
     mTaskRunner.Post([aReceiver](void) { aReceiver(OT_ERROR_NOT_IMPLEMENTED, "Not implemented!"); });
 }
 
-void RcpHost::Leave(const AsyncResultReceiver &aReceiver)
+void RcpHost::Leave(bool aEraseDataset, const AsyncResultReceiver &aReceiver)
 {
-    // TODO: Implement Leave under RCP mode.
-    mTaskRunner.Post([aReceiver](void) { aReceiver(OT_ERROR_NOT_IMPLEMENTED, "Not implemented!"); });
+    otError     error = OT_ERROR_NONE;
+    std::string errorMsg;
+    bool        receiveResultHere = true;
+
+    VerifyOrExit(mInstance != nullptr, error = OT_ERROR_INVALID_STATE, errorMsg = "OT is not initialized");
+    VerifyOrExit(mThreadEnabledState != ThreadEnabledState::kStateDisabling, error = OT_ERROR_BUSY,
+                 errorMsg = "Thread is disabling");
+
+    if (mThreadEnabledState == ThreadEnabledState::kStateDisabled)
+    {
+        ConditionalErasePersistentInfo(aEraseDataset);
+        ExitNow();
+    }
+
+    ThreadDetachGracefully([aEraseDataset, aReceiver, this] {
+        ConditionalErasePersistentInfo(aEraseDataset);
+        if (aReceiver)
+        {
+            aReceiver(OT_ERROR_NONE, "");
+        }
+    });
+
+exit:
+    if (receiveResultHere)
+    {
+        mTaskRunner.Post([aReceiver, error, errorMsg](void) { aReceiver(error, errorMsg); });
+    }
 }
 
 void RcpHost::ScheduleMigration(const otOperationalDatasetTlvs &aPendingOpDatasetTlvs,
@@ -529,15 +555,15 @@ void RcpHost::SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceive
     {
         UpdateThreadEnabledState(ThreadEnabledState::kStateDisabling);
 
-        SuccessOrExit(error = otThreadDetachGracefully(mInstance, DisableThreadAfterDetach, this));
+        ThreadDetachGracefully([this](void) { DisableThreadAfterDetach(); });
         mSetThreadEnabledReceiver = aReceiver;
         receiveResultHere         = false;
     }
 
 exit:
     if (receiveResultHere)
     {
-        mTaskRunner.Post([aReceiver, error, errorMsg](void) { aReceiver(error, errorMsg); });
+        mTaskRunner.Post([aReceiver, error, errorMsg](void) { SafeInvoke(aReceiver, error, errorMsg); });
     }
 }
 
@@ -595,9 +621,36 @@ void RcpHost::SetChannelMaxPowers(const std::vector<ChannelMaxPower> &aChannelMa
 }
 #endif // OTBR_ENABLE_POWER_CALIBRATION
 
-void RcpHost::DisableThreadAfterDetach(void *aContext)
+void RcpHost::ThreadDetachGracefully(const DetachGracefullyCallback &aCallback)
+{
+    mDetachGracefullyCallbacks.push_back(aCallback);
+
+    // Ignores the OT_ERROR_BUSY error if a detach has already been requested
+    OT_UNUSED_VARIABLE(otThreadDetachGracefully(mInstance, ThreadDetachGracefullyCallback, this));
+}
+
+void RcpHost::ThreadDetachGracefullyCallback(void *aContext)
+{
+    static_cast<RcpHost *>(aContext)->ThreadDetachGracefullyCallback();
+}
+
+void RcpHost::ThreadDetachGracefullyCallback(void)
 {
-    static_cast<RcpHost *>(aContext)->DisableThreadAfterDetach();
+    SafeInvokeAndClear(mScheduleMigrationReceiver, OT_ERROR_ABORT, "Aborted by leave/disable operation");
+
+    for (auto &callback : mDetachGracefullyCallbacks)
+    {
+        callback();
+    }
+    mDetachGracefullyCallbacks.clear();
+}
+
+void RcpHost::ConditionalErasePersistentInfo(bool aErase)
+{
+    if (aErase)
+    {
+        OT_UNUSED_VARIABLE(otInstanceErasePersistentInfo(mInstance));
+    }
 }
 
 void RcpHost::DisableThreadAfterDetach(void)

diff --git a/src/ncp/rcp_host.hpp b/src/ncp/rcp_host.hpp
@@ -199,7 +199,7 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro
 
     // Thread Control virtual methods
     void Join(const otOperationalDatasetTlvs &aActiveOpDatasetTlvs, const AsyncResultReceiver &aRecevier) override;
-    void Leave(const AsyncResultReceiver &aRecevier) override;
+    void Leave(bool aEraseDataset, const AsyncResultReceiver &aRecevier) override;
     void ScheduleMigration(const otOperationalDatasetTlvs &aPendingOpDatasetTlvs,
                            const AsyncResultReceiver       aReceiver) override;
     void SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceiver) override;
@@ -231,6 +231,13 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro
             aReceiver = nullptr;
         }
     }
+    static void SafeInvoke(const AsyncResultReceiver &aReceiver, otError aError, const std::string &aErrorInfo = "")
+    {
+        if (aReceiver)
+        {
+            aReceiver(aError, aErrorInfo);
+        }
+    }
 
     static void HandleStateChanged(otChangedFlags aFlags, void *aContext)
     {
@@ -251,7 +258,11 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro
     void        HandleBackboneRouterNdProxyEvent(otBackboneRouterNdProxyEvent aEvent, const otIp6Address *aAddress);
 #endif
 
-    static void DisableThreadAfterDetach(void *aContext);
+    using DetachGracefullyCallback = std::function<void()>;
+    void        ThreadDetachGracefully(const DetachGracefullyCallback &aCallback);
+    static void ThreadDetachGracefullyCallback(void *aContext);
+    void        ThreadDetachGracefullyCallback(void);
+    void        ConditionalErasePersistentInfo(bool aErase);
     void        DisableThreadAfterDetach(void);
     static void SendMgmtPendingSetCallback(otError aError, void *aContext);
     void        SendMgmtPendingSetCallback(otError aError);
@@ -278,6 +289,7 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro
     ThreadEnabledState                      mThreadEnabledState;
     AsyncResultReceiver                     mSetThreadEnabledReceiver;
     AsyncResultReceiver                     mScheduleMigrationReceiver;
+    std::vector<DetachGracefullyCallback>   mDetachGracefullyCallbacks;
 
 #if OTBR_ENABLE_FEATURE_FLAGS
     // The applied FeatureFlagList in ApplyFeatureFlagList call, used for debugging purpose.

diff --git a/src/ncp/thread_host.hpp b/src/ncp/thread_host.hpp
@@ -166,13 +166,13 @@ class ThreadHost : virtual public NetworkProperties
      *    be called.
      * 2. If this device is not in disabled state, OTBR sends Address Release Notification (i.e. ADDR_REL.ntf)
      *    to gracefully detach from the current network and it takes 1 second to finish.
-     * 3. Then Operational Dataset will be removed from persistent storage.
+     * 3. Then Operational Dataset will be removed from persistent storage if @p aEraseDataset is true.
      * 4. If everything goes fine, @p aReceiver will be invoked with OT_ERROR_NONE. Otherwise, other errors
      *    will be passed to @p aReceiver when the error happens.
      *
      * @param[in] aReceiver  A receiver to get the async result of this operation.
      */
-    virtual void Leave(const AsyncResultReceiver &aRecevier) = 0;
+    virtual void Leave(bool aEraseDataset, const AsyncResultReceiver &aRecevier) = 0;
 
     /**
      * This method migrates this device to the new network specified by @p aPendingOpDatasetTlvs.

diff --git a/tests/gtest/test_rcp_host_api.cpp b/tests/gtest/test_rcp_host_api.cpp
@@ -194,6 +194,75 @@ TEST(RcpHostApi, SetCountryCodeWorkCorrectly)
     host.Deinit();
 }
 
+TEST(RcpHostApi, StateChangesCorrectlyAfterLeave)
+{
+    otError                                    error          = OT_ERROR_NONE;
+    std::string                                errorMsg       = "";
+    bool                                       resultReceived = false;
+    otbr::MainloopContext                      mainloop;
+    otbr::Ncp::ThreadHost::AsyncResultReceiver receiver = [&resultReceived, &error,
+                                                           &errorMsg](otError aError, const std::string &aErrorMsg) {
+        resultReceived = true;
+        error          = aError;
+        errorMsg       = aErrorMsg;
+    };
+
+    otbr::Ncp::RcpHost host("wpan0", std::vector<const char *>(), /* aBackboneInterfaceName */ "", /* aDryRun */ false,
+                            /* aEnableAutoAttach */ false);
+
+    // 1. Call Leave when host hasn't been initialized.
+    otbr::MainloopManager::GetInstance().RemoveMainloopProcessor(
+        &host); // Temporarily remove RcpHost because it's not initialized yet.
+    host.Leave(/* aEraseDataset */ true, receiver);
+    MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; });
+    EXPECT_EQ(error, OT_ERROR_INVALID_STATE);
+    EXPECT_STREQ(errorMsg.c_str(), "OT is not initialized");
+    otbr::MainloopManager::GetInstance().AddMainloopProcessor(&host);
+
+    host.Init();
+
+    // 2. Call Leave when disabling Thread.
+    error          = OT_ERROR_NONE;
+    resultReceived = false;
+    host.SetThreadEnabled(false, nullptr);
+    host.Leave(/* aEraseDataset */ true, receiver);
+    MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; });
+    EXPECT_EQ(error, OT_ERROR_BUSY);
+    EXPECT_STREQ(errorMsg.c_str(), "Thread is disabling");
+
+    // 3. Call Leave when Thread is disabled.
+    error          = OT_ERROR_NONE;
+    resultReceived = false;
+    otOperationalDataset     dataset;
+    otOperationalDatasetTlvs datasetTlvs;
+    OT_UNUSED_VARIABLE(otDatasetCreateNewNetwork(ot::FakePlatform::CurrentInstance(), &dataset));
+    otDatasetConvertToTlvs(&dataset, &datasetTlvs);
+    OT_UNUSED_VARIABLE(otDatasetSetActiveTlvs(ot::FakePlatform::CurrentInstance(), &datasetTlvs));
+    host.Leave(/* aEraseDataset */ true, receiver);
+    MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; });
+    EXPECT_EQ(error, OT_ERROR_NONE);
+
+    error = otDatasetGetActive(ot::FakePlatform::CurrentInstance(), &dataset);
+    EXPECT_EQ(error, OT_ERROR_NOT_FOUND);
+
+    // 4. Call Leave when Thread is enabled.
+    error          = OT_ERROR_NONE;
+    resultReceived = false;
+    OT_UNUSED_VARIABLE(otDatasetSetActiveTlvs(ot::FakePlatform::CurrentInstance(), &datasetTlvs));
+    host.SetThreadEnabled(true, nullptr);
+    MainloopProcessUntil(mainloop, /* aTimeoutSec */ 1,
+                         [&host]() { return host.GetDeviceRole() != OT_DEVICE_ROLE_DETACHED; });
+    EXPECT_EQ(host.GetDeviceRole(), OT_DEVICE_ROLE_LEADER);
+    host.Leave(/* aEraseDataset */ false, receiver);
+    MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; });
+    EXPECT_EQ(error, OT_ERROR_NONE);
+
+    error = otDatasetGetActive(ot::FakePlatform::CurrentInstance(), &dataset); // Dataset should still be there.
+    EXPECT_EQ(error, OT_ERROR_NONE);
+
+    host.Deinit();
+}
+
 TEST(RcpHostApi, StateChangesCorrectlyAfterScheduleMigration)
 {
     otError                                    error          = OT_ERROR_NONE;