Fix JointFabricDatastore ownership, lifetime, and sync edge cases

Author: vijs

docs/guides/joint_fabric_guide.md

@@ -136,7 +136,7 @@ $ ./chip-lighting-app --capabilities 0x4 --passcode 11022044 --KVS light_a_kvs
 -   Commission lighting-app
 
 ```
->>> pairing onnetwork 2 11022044
+>>> pairing onnetwork 2 11022044 --regular 1
 ```
 
 Check that a Fabric having `AdminVendorID` set to 0xFFF1 has been installed:
@@ -235,7 +235,7 @@ $ ./chip-lighting-app --capabilities 0x4 --passcode 11022066 --KVS light_b_kvs
 -   Commission lighting-app
 
 ```
->>> pairing onnetwork 22 11022066
+>>> pairing onnetwork 22 11022066 --regular 1
 ```
 
 Check that a Fabric having `AdminVendorID` set to 0xFFF2 has been installed:

examples/jf-admin-app/linux/JFADatastoreSync.cpp

@@ -24,6 +24,10 @@
 #include <app/ConcreteAttributePath.h>
 #include <app/server/Server.h>
 
+#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
+#include <CommissionerMain.h>
+#endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
+
 #include <controller/CHIPCluster.h>
 #include <lib/support/logging/CHIPLogging.h>
 
@@ -197,6 +201,23 @@ void JFAManager::HandleCommissioningCompleteEvent()
 
                 LogErrorOnFailure(Server::GetInstance().GetJointFabricDatastore().AddAdmin(newAdmin));
 
+                if (!mCommissionerInitialized)
+                {
+                    CHIP_ERROR err = InitCommissioner(LinuxDeviceOptions::GetInstance().securedCommissionerPort,
+                                                      LinuxDeviceOptions::GetInstance().unsecuredCommissionerPort, fb.GetFabricId(),
+                                                      fabricIndex);
+                    if (err == CHIP_NO_ERROR)
+                    {
+                        mCommissionerInitialized = true;
+                        ChipLogProgress(JointFabric, "Commissioner mode initialized on commissioned fabric index %u",
+                                        static_cast<unsigned>(fabricIndex));
+                    }
+                    else
+                    {
+                        ChipLogError(JointFabric, "Failed to initialize commissioner mode: %" CHIP_ERROR_FORMAT, err.Format());
+                    }
+                }
+
                 ChipLogProgress(JointFabric, "Joint Fabric Administrator commissioned on fabric index %d", fabricIndex);
             }
         }

examples/jf-admin-app/linux/JFAManager.cpp

@@ -47,3 +47,5 @@
 #define CHIP_DEVICE_CONFIG_DEVICE_NAME "JF Admin"
 
 #define CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE 1
+
+#define CHIP_LINUX_APP_START_COMMISSIONER_AT_BOOT 0

examples/jf-admin-app/linux/include/CHIPProjectAppConfig.h

@@ -21,6 +21,8 @@
 #include <app/server/JointFabricDatastore.h>
 #include <app/server/Server.h>
 #include <lib/core/CHIPError.h>
+#include <map>
+#include <memory>
 
 namespace chip {
 
@@ -73,10 +75,11 @@ class JFADatastoreSync : public app::JointFabricDatastore::Delegate
             void(CHIP_ERROR,
                  const std::vector<app::Clusters::JointFabricDatastore::Structs::DatastoreEndpointBindingEntryStruct::Type> &)>
             onSuccess) override;
-    CHIP_ERROR FetchGroupKeySetList(
-        NodeId nodeId,
-        std::function<void(CHIP_ERROR,
-                           const std::vector<app::Clusters::JointFabricDatastore::Structs::DatastoreGroupKeySetStruct::Type> &)>
+    CHIP_ERROR FetchGroupKeySetList(NodeId nodeId,
+                                    std::function<void(CHIP_ERROR, const std::vector<uint16_t> &)> onSuccess) override;
+    CHIP_ERROR FetchGroupKeySet(
+        NodeId nodeId, uint16_t groupKeySetID,
+        std::function<void(CHIP_ERROR, const app::Clusters::JointFabricDatastore::Structs::DatastoreGroupKeySetStruct::Type &)>
             onSuccess) override;
     CHIP_ERROR FetchACLList(
         NodeId nodeId,
@@ -90,6 +93,23 @@ class JFADatastoreSync : public app::JointFabricDatastore::Delegate
     static JFADatastoreSync sJFDS;
 
     Server * mServer = nullptr;
+
+    // Map to hold in-flight device pairing commands, keyed by unique command token.
+    // This allows multiple concurrent commands for the same node to coexist safely.
+    std::map<uint64_t, std::shared_ptr<void>> mInFlightCommands;
+    uint64_t mNextInFlightCommandToken = 1;
+
+    uint64_t AllocateInFlightCommandToken() { return mNextInFlightCommandToken++; }
+
+    // Helper to store command and ensure it stays alive until callbacks complete.
+    template <typename T>
+    void StoreInFlightCommand(uint64_t token, std::shared_ptr<T> command)
+    {
+        mInFlightCommands[token] = std::static_pointer_cast<void>(command);
+    }
+
+    // Helper to remove command after callbacks complete.
+    void RemoveInFlightCommand(uint64_t token) { mInFlightCommands.erase(token); }
 };
 
 inline JFADatastoreSync & JFADSync(void)

examples/jf-admin-app/linux/include/JFADatastoreSync.h

@@ -81,7 +81,8 @@ class JFAManager : public app::JointFabricAdministrator::Delegate
     EndpointId peerAdminJFAdminClusterEndpointId = kInvalidEndpointId;
     Crypto::P256PublicKey peerAdminICACPubKey;
     uint8_t mICACBuffer[Credentials::kMaxDERCertLength];
-    size_t mICACBufferLen = 0;
+    size_t mICACBufferLen         = 0;
+    bool mCommissionerInitialized = false;
 
     void ConnectToNode(ScopedNodeId scopedNodeId, OnConnectedAction onConnectedAction);
     CHIP_ERROR SendCommissioningComplete();

examples/jf-admin-app/linux/include/JFAManager.h

@@ -122,7 +122,7 @@ CHIP_ERROR PairingCommand::RunCommand()
                      ChipLogValueX64(anchorNodeId));
         return CHIP_ERROR_BAD_REQUEST;
     }
-    else
+    else if (!mRegularDevice.ValueOr(false))
     {
         // Skip commissioning complete for JCM and other device commissioning methods but not Anchor Administrator commissioning.
         mSkipCommissioningComplete = MakeOptional(true);
@@ -682,7 +682,11 @@ void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err)
 {
     if (err == CHIP_NO_ERROR)
     {
-        if (!mSkipCommissioningComplete.ValueOr(false))
+        if (mRegularDevice.ValueOr(false))
+        {
+            ChipLogProgress(JointFabric, "Device (nodeId=%ld) commissioned with success", nodeId);
+        }
+        else if (!mSkipCommissioningComplete.ValueOr(false))
         {
             ChipLogProgress(JointFabric, "Anchor Administrator (nodeId=%ld) commissioned with success", nodeId);
             TEMPORARY_RETURN_IGNORED SetAnchorNodeId(nodeId);

examples/jf-control-app/commands/pairing/PairingCommand.cpp

@@ -103,6 +103,7 @@ class PairingCommand : public CHIPCommand,
                     "If set, a LIT ICD that is commissioned will be requested to stay active for this many milliseconds");
         AddArgument("anchor", 0, 1, &mAnchor, "If set to true then a NOC with Anchor and Administrator CAT is issued");
         AddArgument("jcm", 0, 1, &mJCM, "Set it to true in order to commission a Joint Fabric Administrator");
+        AddArgument("regular", 0, 1, &mRegularDevice, "Set it to true to commission a regular device");
         switch (networkType)
         {
         case PairingNetworkType::None:
@@ -296,6 +297,7 @@ class PairingCommand : public CHIPCommand,
     chip::Optional<bool> mSkipCommissioningComplete;
     chip::Optional<bool> mAnchor;
     chip::Optional<bool> mJCM;
+    chip::Optional<bool> mRegularDevice;
     chip::Optional<bool> mBypassAttestationVerifier;
     chip::Optional<std::vector<uint32_t>> mCASEAuthTags;
     chip::Optional<char *> mCountryCode;

examples/jf-control-app/commands/pairing/PairingCommand.h

@@ -59,6 +59,10 @@
 #include <ControllerShellCommands.h>
 #endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
 
+#ifndef CHIP_LINUX_APP_START_COMMISSIONER_AT_BOOT
+#define CHIP_LINUX_APP_START_COMMISSIONER_AT_BOOT 1
+#endif
+
 #if defined(ENABLE_CHIP_SHELL)
 #include <CommissioneeShellCommands.h>
 #include <lib/shell/Engine.h> // nogncheck
@@ -1049,11 +1053,13 @@ void ChipLinuxAppMainLoop(chip::ServerInitParams & initParams, AppMainLoopImplem
     PrintOnboardingCodes(LinuxDeviceOptions::GetInstance().payload);
 
 #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
+#if CHIP_LINUX_APP_START_COMMISSIONER_AT_BOOT
     ChipLogProgress(AppServer, "Starting commissioner");
     VerifyOrReturn(InitCommissioner(LinuxDeviceOptions::GetInstance().securedCommissionerPort,
                                     LinuxDeviceOptions::GetInstance().unsecuredCommissionerPort,
                                     LinuxDeviceOptions::GetInstance().commissionerFabricId) == CHIP_NO_ERROR);
     ChipLogProgress(AppServer, "Started commissioner");
+#endif // CHIP_LINUX_APP_START_COMMISSIONER_AT_BOOT
 #if defined(ENABLE_CHIP_SHELL)
     Shell::RegisterControllerCommands();
 #endif // defined(ENABLE_CHIP_SHELL)

examples/platform/linux/AppMain.cpp

@@ -123,7 +123,8 @@ NodeId gLocalId = kMaxOperationalNodeId;
 Credentials::GroupDataProviderImpl gGroupDataProvider;
 Crypto::RawKeySessionKeystore gSessionKeystore;
 
-CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort, FabricId fabricId)
+CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort, FabricId fabricId,
+                            FabricIndex commissionerFabricIndex)
 {
     Controller::FactoryInitParams factoryParams;
     Controller::SetupParams params;
@@ -149,9 +150,17 @@ CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort, F
     params.controllerVendorId = static_cast<VendorId>(vendorId);
 
     ReturnErrorOnFailure(gOpCredsIssuer.Initialize(gServerStorage));
-    if (fabricId != kUndefinedFabricId)
+
+    if (commissionerFabricIndex == kUndefinedFabricIndex && fabricId != kUndefinedFabricId)
     {
-        gOpCredsIssuer.SetFabricIdForNextNOCRequest(fabricId);
+        for (const auto & fb : Server::GetInstance().GetFabricTable())
+        {
+            if (fb.GetFabricId() == fabricId)
+            {
+                commissionerFabricIndex = fb.GetFabricIndex();
+                break;
+            }
+        }
     }
 
     // No need to explicitly set the UDC port since we will use default
@@ -179,17 +188,34 @@ CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort, F
     Platform::ScopedMemoryBuffer<uint8_t> rcac;
     VerifyOrReturnError(rcac.Alloc(Controller::kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY);
     MutableByteSpan rcacSpan(rcac.Get(), Controller::kMaxCHIPDERCertLength);
-
     Crypto::P256Keypair ephemeralKey;
-    ReturnErrorOnFailure(ephemeralKey.Initialize(Crypto::ECPKeyTarget::ECDSA));
 
-    ReturnErrorOnFailure(gOpCredsIssuer.GenerateNOCChainAfterValidation(gLocalId, /* fabricId = */ 1, chip::kUndefinedCATs,
-                                                                        ephemeralKey.Pubkey(), rcacSpan, icacSpan, nocSpan));
+    if (commissionerFabricIndex != kUndefinedFabricIndex)
+    {
+        params.fabricIndex.SetValue(commissionerFabricIndex);
+        params.removeFromFabricTableOnShutdown = false;
+
+        ChipLogProgress(Support, " ----- Commissioner reusing existing fabric index %u",
+                        static_cast<unsigned>(commissionerFabricIndex));
+    }
+    else
+    {
+        const FabricId commissionerFabricId = (fabricId == kUndefinedFabricId) ? static_cast<FabricId>(1) : fabricId;
+        if (commissionerFabricId != kUndefinedFabricId)
+        {
+            gOpCredsIssuer.SetFabricIdForNextNOCRequest(commissionerFabricId);
+        }
+
+        ReturnErrorOnFailure(ephemeralKey.Initialize(Crypto::ECPKeyTarget::ECDSA));
+
+        ReturnErrorOnFailure(gOpCredsIssuer.GenerateNOCChainAfterValidation(gLocalId, commissionerFabricId, chip::kUndefinedCATs,
+                                                                            ephemeralKey.Pubkey(), rcacSpan, icacSpan, nocSpan));
 
-    params.operationalKeypair = &ephemeralKey;
-    params.controllerRCAC     = rcacSpan;
-    params.controllerICAC     = icacSpan;
-    params.controllerNOC      = nocSpan;
+        params.operationalKeypair = &ephemeralKey;
+        params.controllerRCAC     = rcacSpan;
+        params.controllerICAC     = icacSpan;
+        params.controllerNOC      = nocSpan;
+    }
 
     params.defaultCommissioner      = &gAutoCommissioner;
     params.enableServerInteractions = true;
@@ -227,7 +253,8 @@ CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort, F
 
     ChipLogProgress(Support,
                     "InitCommissioner nodeId=0x" ChipLogFormatX64 " fabric.fabricId=0x" ChipLogFormatX64 " fabricIndex=0x%x",
-                    ChipLogValueX64(gCommissioner.GetNodeId()), ChipLogValueX64(fabricId), static_cast<unsigned>(fabricIndex));
+                    ChipLogValueX64(gCommissioner.GetNodeId()), ChipLogValueX64(gCommissioner.GetFabricId()),
+                    static_cast<unsigned>(fabricIndex));
 
     return CHIP_NO_ERROR;
 }