Refactor protocol helper.

Simplify and improve data structures that track protocol client connections. The prior code could not store pg or repo clients but not both. We don't have a need for that yet, but tracking clients only by hostIdx was not flexible for some upcoming improvements. It is important to be able to identify and free clients very precisely.

In general this code should be easier to understand and removes duplicated code for local/remote clients.

src/command/backup/backup.c

@@ -2817,7 +2817,7 @@ cmdBackup(void)
         if (backupData->dbStandby != NULL)
         {
             dbFree(backupData->dbStandby);
-            protocolRemoteFree(backupData->pgIdxStandby);
+            protocolHelperFree(protocolRemoteGet(protocolStorageTypePg, backupData->pgIdxStandby, false));
         }
 
         // Stop the backup
@@ -2841,7 +2841,7 @@ cmdBackup(void)
         // The primary protocol connection won't be used anymore so free it. This needs to happen after backupArchiveCheckCopy() so
         // the backup lock is held on the remote which allows conditional archiving based on the backup lock. Any further access to
         // the primary storage object may result in an error (likely eof).
-        protocolRemoteFree(backupData->pgIdxPrimary);
+        protocolHelperFree(protocolRemoteGet(protocolStorageTypePg, backupData->pgIdxPrimary, false));
 
         // Complete the backup
         LOG_INFO_FMT("new backup label = %s", strZ(manifestData(manifest)->backupLabel));

src/db/helper.c

@@ -38,7 +38,7 @@ dbGetIdx(const unsigned int pgIdx)
                 NULL, storagePgIdx(pgIdx), applicationName);
         }
         else
-            result = dbNew(NULL, protocolRemoteGet(protocolStorageTypePg, pgIdx), storagePgIdx(pgIdx), applicationName);
+            result = dbNew(NULL, protocolRemoteGet(protocolStorageTypePg, pgIdx, true), storagePgIdx(pgIdx), applicationName);
 
         dbMove(result, memContextPrior());
     }

src/protocol/helper.c

@@ -33,8 +33,18 @@ STRING_EXTERN(PROTOCOL_SERVICE_REMOTE_STR,                          PROTOCOL_SER
 /***********************************************************************************************************************************
 Local variables
 ***********************************************************************************************************************************/
+typedef enum
+{
+    protocolClientLocal = STRID5("local", 0xc08dec0),
+    protocolClientRemote = STRID5("remote", 0xb47b4b20),
+} ProtocolClientType;
+
 typedef struct ProtocolHelperClient
 {
+    ProtocolClientType type;                                        // Client type
+    ProtocolStorageType storageType;                                // Storage type
+    unsigned int hostIdx;                                           // Host index
+    unsigned int processId;                                         // Process id displayed in logs
     Exec *exec;                                                     // Executed client
     IoClient *ioClient;                                             // Io client, e.g. TlsClient
     IoSession *ioSession;                                           // Io session, e.g. TlsSession
@@ -44,12 +54,7 @@ typedef struct ProtocolHelperClient
 static struct
 {
     MemContext *memContext;                                         // Mem context for protocol helper
-
+    List *clientList;                                               // Client List
-    unsigned int clientRemoteSize;                                  // Remote clients
-    ProtocolHelperClient *clientRemote;
-
-    unsigned int clientLocalSize;                                   // Local clients
-    ProtocolHelperClient *clientLocal;
 } protocolHelper;
 
 /***********************************************************************************************************************************
@@ -67,6 +72,7 @@ protocolHelperInit(void)
             MEM_CONTEXT_NEW_BEGIN("ProtocolHelper", .childQty = MEM_CONTEXT_QTY_MAX, .allocQty = MEM_CONTEXT_QTY_MAX)
             {
                 protocolHelper.memContext = MEM_CONTEXT_NEW();
+                protocolHelper.clientList = lstNewP(sizeof(ProtocolHelperClient));
             }
             MEM_CONTEXT_NEW_END();
         }
@@ -74,6 +80,32 @@ protocolHelperInit(void)
     }
 }
 
+/***********************************************************************************************************************************
+Init local mem context and data structure
+***********************************************************************************************************************************/
+static ProtocolHelperClient *
+protocolHelperClientGet(ProtocolClientType type, ProtocolStorageType storageType, unsigned int hostIdx, unsigned int processId)
+{
+    FUNCTION_TEST_BEGIN();
+        FUNCTION_TEST_PARAM(STRING_ID, type);
+        FUNCTION_TEST_PARAM(STRING_ID, storageType);
+        FUNCTION_TEST_PARAM(UINT, hostIdx);
+        FUNCTION_TEST_PARAM(UINT, processId);
+    FUNCTION_TEST_END();
+
+    ASSERT(protocolHelper.clientList != NULL);
+
+    for (unsigned int clientIdx = 0; clientIdx < lstSize(protocolHelper.clientList); clientIdx++)
+    {
+        ProtocolHelperClient *const match = lstGet(protocolHelper.clientList, clientIdx);
+
+        if (match->type == type && match->storageType == storageType && match->hostIdx == hostIdx && match->processId == processId)
+            FUNCTION_TEST_RETURN_P(VOID, match);
+    }
+
+    FUNCTION_TEST_RETURN_P(VOID, NULL);
+}
+
 /**********************************************************************************************************************************/
 FN_EXTERN bool
 repoIsLocal(const unsigned int repoIdx)
@@ -244,30 +276,24 @@ protocolLocalGet(const ProtocolStorageType protocolStorageType, const unsigned i
 
     protocolHelperInit();
 
-    // Allocate the client cache
-    if (protocolHelper.clientLocalSize == 0)
-    {
-        MEM_CONTEXT_BEGIN(protocolHelper.memContext)
-        {
-            protocolHelper.clientLocalSize = cfgOptionUInt(cfgOptProcessMax) + 1;
-            protocolHelper.clientLocal = memNew(protocolHelper.clientLocalSize * sizeof(ProtocolHelperClient));
-
-            for (unsigned int clientIdx = 0; clientIdx < protocolHelper.clientLocalSize; clientIdx++)
-                protocolHelper.clientLocal[clientIdx] = (ProtocolHelperClient){.exec = NULL};
-        }
-        MEM_CONTEXT_END();
-    }
-
-    ASSERT(processId <= protocolHelper.clientLocalSize);
-
     // Create protocol object
-    ProtocolHelperClient *protocolHelperClient = &protocolHelper.clientLocal[processId - 1];
+    ProtocolHelperClient *protocolHelperClient = protocolHelperClientGet(
+        protocolClientLocal, protocolStorageType, hostIdx, processId);
 
-    if (protocolHelperClient->client == NULL)
+    if (protocolHelperClient == NULL)
     {
         MEM_CONTEXT_BEGIN(protocolHelper.memContext)
         {
-            protocolLocalExec(protocolHelperClient, protocolStorageType, hostIdx, processId);
+            ProtocolHelperClient protocolHelperClientAdd =
+            {
+                .type = protocolClientLocal,
+                .storageType = protocolStorageType,
+                .hostIdx = hostIdx,
+                .processId = processId,
+            };
+
+            protocolLocalExec(&protocolHelperClientAdd, protocolStorageType, hostIdx, processId);
+            protocolHelperClient = lstAdd(protocolHelper.clientList, &protocolHelperClientAdd);
         }
         MEM_CONTEXT_END();
 
@@ -289,8 +315,6 @@ protocolHelperClientFree(ProtocolHelperClient *const protocolHelperClient)
         FUNCTION_LOG_PARAM_P(VOID, protocolHelperClient);
     FUNCTION_LOG_END();
 
-    if (protocolHelperClient->client != NULL)
-    {
     // Try to shutdown the protocol but only warn on error
     TRY_BEGIN()
     {
@@ -317,29 +341,6 @@ protocolHelperClientFree(ProtocolHelperClient *const protocolHelperClient)
     ioSessionFree(protocolHelperClient->ioSession);
     ioClientFree(protocolHelperClient->ioClient);
 
-        protocolHelperClient->ioSession = NULL;
-        protocolHelperClient->ioClient = NULL;
-        protocolHelperClient->client = NULL;
-        protocolHelperClient->exec = NULL;
-    }
-
-    FUNCTION_LOG_RETURN_VOID();
-}
-
-/**********************************************************************************************************************************/
-FN_EXTERN void
-protocolLocalFree(const unsigned int processId)
-{
-    FUNCTION_LOG_BEGIN(logLevelDebug);
-        FUNCTION_LOG_PARAM(UINT, processId);
-    FUNCTION_LOG_END();
-
-    if (protocolHelper.clientLocal != NULL)
-    {
-        ASSERT(processId <= protocolHelper.clientLocalSize);
-        protocolHelperClientFree(&protocolHelper.clientLocal[processId - 1]);
-    }
-
     FUNCTION_LOG_RETURN_VOID();
 }
 
@@ -585,7 +586,12 @@ protocolRemoteParam(const ProtocolStorageType protocolStorageType, const unsigne
 
         // Set repo default so the remote only operates on a single repo
         if (protocolStorageType == protocolStorageTypeRepo)
+        {
             kvPut(optionReplace, VARSTRDEF(CFGOPT_REPO), VARUINT(cfgOptionGroupIdxToKey(cfgOptGrpRepo, hostIdx)));
+        }
+        // Else remove repo selection when the remote is pg
+        else
+            kvPut(optionReplace, VARSTRDEF(CFGOPT_REPO), NULL);
 
         // Add the process id if not set. This means that the remote is being started from the main process and should always get a
         // process id of 0.
@@ -807,11 +813,12 @@ protocolRemoteExec(
 }
 
 FN_EXTERN ProtocolClient *
-protocolRemoteGet(const ProtocolStorageType protocolStorageType, const unsigned int hostIdx)
+protocolRemoteGet(const ProtocolStorageType protocolStorageType, const unsigned int hostIdx, const bool create)
 {
     FUNCTION_LOG_BEGIN(logLevelDebug);
         FUNCTION_LOG_PARAM(STRING_ID, protocolStorageType);
         FUNCTION_LOG_PARAM(UINT, hostIdx);
+        FUNCTION_LOG_PARAM(BOOL, create);
     FUNCTION_LOG_END();
 
     // Is this a repo remote?
@@ -819,34 +826,29 @@ protocolRemoteGet(const ProtocolStorageType protocolStorageType, const unsigned
 
     protocolHelperInit();
 
-    // Allocate the client cache
-    if (protocolHelper.clientRemoteSize == 0)
-    {
-        MEM_CONTEXT_BEGIN(protocolHelper.memContext)
-        {
-            protocolHelper.clientRemoteSize = cfgOptionGroupIdxTotal(isRepo ? cfgOptGrpRepo : cfgOptGrpPg) + 1;
-            protocolHelper.clientRemote = memNew(protocolHelper.clientRemoteSize * sizeof(ProtocolHelperClient));
-
-            for (unsigned int clientIdx = 0; clientIdx < protocolHelper.clientRemoteSize; clientIdx++)
-                protocolHelper.clientRemote[clientIdx] = (ProtocolHelperClient){.exec = NULL};
-        }
-        MEM_CONTEXT_END();
-    }
-
     // Determine protocol id for the remote. If the process option is set then use that since we want the remote protocol id to
     // match the local protocol id. Otherwise set to 0 since the remote is being started from a main process and there should only
     // be one remote per host.
     const unsigned int processId = cfgOptionTest(cfgOptProcess) ? cfgOptionUInt(cfgOptProcess) : 0;
-    CHECK(AssertError, hostIdx < protocolHelper.clientRemoteSize, "invalid host");
 
     // Create protocol object
-    ProtocolHelperClient *const protocolHelperClient = &protocolHelper.clientRemote[hostIdx];
+    ProtocolHelperClient *protocolHelperClient = protocolHelperClientGet(
+        protocolClientRemote, protocolStorageType, hostIdx, processId);
 
-    if (protocolHelperClient->client == NULL)
+    if (protocolHelperClient == NULL && create)
     {
         MEM_CONTEXT_BEGIN(protocolHelper.memContext)
         {
-            protocolRemoteExec(protocolHelperClient, protocolStorageType, hostIdx, processId);
+            ProtocolHelperClient protocolHelperClientAdd =
+            {
+                .type = protocolClientRemote,
+                .storageType = protocolStorageType,
+                .hostIdx = hostIdx,
+                .processId = processId,
+            };
+
+            protocolRemoteExec(&protocolHelperClientAdd, protocolStorageType, hostIdx, processId);
+            protocolHelperClient = lstAdd(protocolHelper.clientList, &protocolHelperClientAdd);
 
             // Send noop to catch initialization errors
             protocolClientNoOp(protocolHelperClient->client);
@@ -871,19 +873,32 @@ protocolRemoteGet(const ProtocolStorageType protocolStorageType, const unsigned
         MEM_CONTEXT_END();
     }
 
-    FUNCTION_LOG_RETURN(PROTOCOL_CLIENT, protocolHelperClient->client);
+    FUNCTION_LOG_RETURN(PROTOCOL_CLIENT, protocolHelperClient != NULL ? protocolHelperClient->client : NULL);
 }
 
 /**********************************************************************************************************************************/
 FN_EXTERN void
-protocolRemoteFree(const unsigned int hostIdx)
+protocolHelperFree(ProtocolClient *const client)
 {
     FUNCTION_LOG_BEGIN(logLevelDebug);
-        FUNCTION_LOG_PARAM(UINT, hostIdx);
+        FUNCTION_LOG_PARAM(PROTOCOL_CLIENT, client);
     FUNCTION_LOG_END();
 
-    if (protocolHelper.clientRemote != NULL)
+    if (client != NULL && protocolHelper.clientList != NULL)
-        protocolHelperClientFree(&protocolHelper.clientRemote[hostIdx]);
+    {
+        for (unsigned int clientIdx = 0; clientIdx < lstSize(protocolHelper.clientList); clientIdx++)
+        {
+            ProtocolHelperClient *const match = lstGet(protocolHelper.clientList, clientIdx);
+
+            if (match->client == client)
+            {
+                protocolHelperClientFree(match);
+                lstRemoveIdx(protocolHelper.clientList, clientIdx);
+
+                break;
+            }
+        }
+    }
 
     FUNCTION_LOG_RETURN_VOID();
 }
@@ -894,12 +909,14 @@ protocolKeepAlive(void)
 {
     FUNCTION_LOG_VOID(logLevelTrace);
 
-    if (protocolHelper.memContext != NULL)
+    if (protocolHelper.clientList)
     {
-        for (unsigned int clientIdx = 0; clientIdx < protocolHelper.clientRemoteSize; clientIdx++)
+        for (unsigned int clientIdx = 0; clientIdx < lstSize(protocolHelper.clientList); clientIdx++)
         {
-            if (protocolHelper.clientRemote[clientIdx].client != NULL)
+            ProtocolHelperClient *const match = lstGet(protocolHelper.clientList, clientIdx);
-                protocolClientNoOp(protocolHelper.clientRemote[clientIdx].client);
+
+            if (match->type == protocolClientRemote)
+                protocolClientNoOp(match->client);
         }
     }
 
@@ -912,15 +929,13 @@ protocolFree(void)
 {
     FUNCTION_LOG_VOID(logLevelTrace);
 
-    if (protocolHelper.memContext != NULL)
+    if (protocolHelper.clientList != NULL)
     {
-        // Free remotes
+        while (lstSize(protocolHelper.clientList) > 0)
-        for (unsigned int clientIdx = 0; clientIdx < protocolHelper.clientRemoteSize; clientIdx++)
+        {
-            protocolRemoteFree(clientIdx);
+            protocolHelperClientFree(lstGet(protocolHelper.clientList, 0));
-
+            lstRemoveIdx(protocolHelper.clientList, 0);
-        // Free locals
+        }
-        for (unsigned int clientIdx = 1; clientIdx <= protocolHelper.clientLocalSize; clientIdx++)
-            protocolLocalFree(clientIdx);
     }
 
     FUNCTION_LOG_RETURN_VOID();

src/protocol/helper.h

@@ -51,14 +51,11 @@ FN_EXTERN void protocolKeepAlive(void);
 // Local protocol client
 FN_EXTERN ProtocolClient *protocolLocalGet(ProtocolStorageType protocolStorageType, unsigned int hostId, unsigned int protocolId);
 
-// Free (shutdown) a local
-FN_EXTERN void protocolLocalFree(unsigned int protocolId);
-
 // Remote protocol client
-FN_EXTERN ProtocolClient *protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int hostId);
+FN_EXTERN ProtocolClient *protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int hostId, bool create);
 
-// Free (shutdown) a remote
+// Free (shutdown) a local/remote client
-FN_EXTERN void protocolRemoteFree(unsigned int hostId);
+FN_EXTERN void protocolHelperFree(ProtocolClient *client);
 
 // Initialize a server
 FN_EXTERN ProtocolServer *protocolServer(IoServer *const tlsServer, IoSession *const socketSession);

src/protocol/parallel.c

@@ -199,6 +199,7 @@ protocolParallelProcess(ProtocolParallel *const this)
                 {
                     // Get a new job
                     ProtocolParallelJob *const job = this->callbackFunction(this->callbackData, clientIdx);
+                    ProtocolClient *const client = *(ProtocolClient **)lstGet(this->clientList, clientIdx);
 
                     // If a new job was found
                     if (job != NULL)
@@ -208,8 +209,7 @@ protocolParallelProcess(ProtocolParallel *const this)
 
                         // Put command
                         ProtocolClientSession *const session = protocolClientSessionNewP(
-                            *(ProtocolClient **)lstGet(this->clientList, clientIdx), protocolParallelJobCommand(job),
+                            client, protocolParallelJobCommand(job), .async = true);
-                            .async = true);
                         protocolClientSessionRequestAsyncP(session, .param = protocolParallelJobParam(job));
 
                         // Set client id and running state
@@ -221,7 +221,7 @@ protocolParallelProcess(ProtocolParallel *const this)
                     }
                     // Else no more jobs for this client so free it
                     else
-                        protocolLocalFree(clientIdx + 1);
+                        protocolHelperFree(client);
                 }
                 MEM_CONTEXT_END();
             }

src/storage/helper.c

@@ -190,7 +190,7 @@ storagePgGet(const unsigned int pgIdx, const bool write)
     {
         result = storageRemoteNew(
             STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, write, 0, NULL,
-            protocolRemoteGet(protocolStorageTypePg, pgIdx), cfgOptionUInt(cfgOptCompressLevelNetwork));
+            protocolRemoteGet(protocolStorageTypePg, pgIdx, true), cfgOptionUInt(cfgOptCompressLevelNetwork));
     }
     // Use Posix storage
     else
@@ -361,7 +361,7 @@ storageRepoGet(const unsigned int repoIdx, const bool write)
     {
         result = storageRemoteNew(
             STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, write, storageRepoTargetTime(), storageRepoPathExpression,
-            protocolRemoteGet(protocolStorageTypeRepo, repoIdx), cfgOptionUInt(cfgOptCompressLevelNetwork));
+            protocolRemoteGet(protocolStorageTypeRepo, repoIdx, true), cfgOptionUInt(cfgOptCompressLevelNetwork));
     }
     // Use local storage
     else

test/src/common/harnessProtocol.c

@@ -46,45 +46,21 @@ hrnProtocolClientCleanup(void)
 {
     FUNCTION_LOG_VOID(logLevelDebug);
 
-    if (protocolHelper.memContext != NULL)
+    if (protocolHelper.clientList != NULL)
     {
         // Cleanup remotes
-        for (unsigned int clientIdx = 0; clientIdx < protocolHelper.clientRemoteSize; clientIdx++)
+        while (lstSize(protocolHelper.clientList) > 0)
         {
-            // Cleanup remote client
+            ProtocolHelperClient *const clientHelper = lstGet(protocolHelper.clientList, 0);
-            if (protocolHelper.clientRemote[clientIdx].client != NULL)
-            {
-                memContextCallbackClear(objMemContext(protocolHelper.clientRemote[clientIdx].client));
-                protocolClientFree(protocolHelper.clientRemote[clientIdx].client);
-                protocolHelper.clientRemote[clientIdx].client = NULL;
-            }
 
-            // Cleanup remote exec
+            memContextCallbackClear(objMemContext(clientHelper->client));
-            if (protocolHelper.clientRemote[clientIdx].exec != NULL)
+            protocolClientFree(clientHelper->client);
-            {
+            lstRemoveIdx(protocolHelper.clientList, 0);
-                memContextCallbackClear(objMemContext(protocolHelper.clientRemote[clientIdx].exec));
-                execFree(protocolHelper.clientRemote[clientIdx].exec);
-                protocolHelper.clientRemote[clientIdx].exec = NULL;
-            }
-        }
 
-        // Cleanup locals
+            if (clientHelper->exec != NULL)
-        for (unsigned int clientIdx = 0; clientIdx < protocolHelper.clientLocalSize; clientIdx++)
             {
-            // Cleanup local client
+                memContextCallbackClear(objMemContext(clientHelper->exec));
-            if (protocolHelper.clientLocal[clientIdx].client != NULL)
+                execFree(clientHelper->exec);
-            {
-                memContextCallbackClear(objMemContext(protocolHelper.clientLocal[clientIdx].client));
-                protocolClientFree(protocolHelper.clientLocal[clientIdx].client);
-                protocolHelper.clientLocal[clientIdx].client = NULL;
-            }
-
-            // Cleanup local exec
-            if (protocolHelper.clientLocal[clientIdx].exec != NULL)
-            {
-                memContextCallbackClear(objMemContext(protocolHelper.clientLocal[clientIdx].exec));
-                execFree(protocolHelper.clientLocal[clientIdx].exec);
-                protocolHelper.clientLocal[clientIdx].exec = NULL;
             }
         }
     }

test/src/module/command/serverTest.c

@@ -48,29 +48,32 @@ testRun(void)
                 HRN_CFG_LOAD(cfgCmdArchiveGet, argList);
 
                 // Client 1
+                ProtocolClient *clientRemote = protocolRemoteGet(protocolStorageTypeRepo, 0, true);
                 const Storage *storageRemote = NULL;
                 TEST_ASSIGN(
                     storageRemote,
                     storageRemoteNew(
                         STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, true, 0, NULL,
-                        protocolRemoteGet(protocolStorageTypeRepo, 0), cfgOptionUInt(cfgOptCompressLevelNetwork)),
+                        protocolRemoteGet(protocolStorageTypeRepo, 0, true), cfgOptionUInt(cfgOptCompressLevelNetwork)),
                     "new storage 1");
 
                 HRN_STORAGE_PUT_Z(storageRemote, "client1.txt", "CLIENT1");
 
-                TEST_RESULT_VOID(protocolRemoteFree(0), "free client 1");
+                TEST_RESULT_VOID(protocolHelperFree(clientRemote), "free client 1");
 
                 // Client 2
+                clientRemote = protocolRemoteGet(protocolStorageTypeRepo, 0, true);
+
                 TEST_ASSIGN(
                     storageRemote,
                     storageRemoteNew(
                         STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, true, 0, NULL,
-                        protocolRemoteGet(protocolStorageTypeRepo, 0), cfgOptionUInt(cfgOptCompressLevelNetwork)),
+                        protocolRemoteGet(protocolStorageTypeRepo, 0, true), cfgOptionUInt(cfgOptCompressLevelNetwork)),
                     "new storage 2");
 
                 HRN_STORAGE_PUT_Z(storageRemote, "client2.txt", "CLIENT2");
 
-                TEST_RESULT_VOID(protocolRemoteFree(0), "free client 2");
+                TEST_RESULT_VOID(protocolHelperFree(clientRemote), "free client 2");
 
                 // Notify parent on exit
                 HRN_FORK_CHILD_NOTIFY_PUT();
@@ -95,17 +98,19 @@ testRun(void)
                 HRN_CFG_LOAD(cfgCmdBackup, argList, .role = cfgCmdRoleLocal);
 
                 // Client 3
+                ProtocolClient *const clientRemote = protocolRemoteGet(protocolStorageTypePg, 0, true);
                 const Storage *storageRemote = NULL;
+
                 TEST_ASSIGN(
                     storageRemote,
                     storageRemoteNew(
                         STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, true, 0, NULL,
-                        protocolRemoteGet(protocolStorageTypePg, 0), cfgOptionUInt(cfgOptCompressLevelNetwork)),
+                        protocolRemoteGet(protocolStorageTypePg, 0, true), cfgOptionUInt(cfgOptCompressLevelNetwork)),
                     "new storage 3");
 
                 HRN_STORAGE_PUT_Z(storageRemote, "client3.txt", "CLIENT3");
 
-                TEST_RESULT_VOID(protocolRemoteFree(0), "free client 3");
+                TEST_RESULT_VOID(protocolHelperFree(clientRemote), "free client 3");
 
                 // Notify parent on exit
                 HRN_FORK_CHILD_NOTIFY_PUT();

test/src/module/protocol/protocolTest.c

@@ -1272,12 +1272,7 @@ testRun(void)
         // -------------------------------------------------------------------------------------------------------------------------
         TEST_TITLE("call remote free before any remotes exist");
 
-        TEST_RESULT_VOID(protocolRemoteFree(1), "free remote (non exist)");
+        TEST_RESULT_VOID(protocolHelperFree(NULL), "free remote (non-existing)");
-
-        // -------------------------------------------------------------------------------------------------------------------------
-        TEST_TITLE("free local that does not exist");
-
-        TEST_RESULT_VOID(protocolLocalFree(2), "free");
 
         // -------------------------------------------------------------------------------------------------------------------------
         TEST_TITLE("call keep alive free before any remotes exist");
@@ -1299,10 +1294,26 @@ testRun(void)
 
         TEST_RESULT_VOID(protocolFree(), "free protocol objects before anything has been created");
 
-        TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 0), "get remote protocol");
+        TEST_RESULT_PTR(protocolRemoteGet(protocolStorageTypeRepo, 0, false), NULL, "get remote cached protocol (no create)");
-        TEST_RESULT_PTR(protocolRemoteGet(protocolStorageTypeRepo, 0), client, "get remote cached protocol");
+        TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 0, true), "get remote protocol");
-        TEST_RESULT_PTR(protocolHelper.clientRemote[0].client, client, "check position in cache");
+        TEST_RESULT_PTR(protocolRemoteGet(protocolStorageTypeRepo, 0, true), client, "get remote cached protocol");
+
+        TEST_RESULT_PTR(protocolHelperClientGet(protocolClientLocal, protocolStorageTypePg, 0, 0), NULL, "cache miss");
+        TEST_RESULT_PTR(protocolHelperClientGet(protocolClientRemote, protocolStorageTypePg, 0, 0), NULL, "cache miss");
+        TEST_RESULT_PTR(protocolHelperClientGet(protocolClientRemote, protocolStorageTypeRepo, 0, 1), NULL, "cache miss");
+
+        // Add a fake local protocol to ensure noops are only sent to remotes
+        ProtocolHelperClient protocolHelperClientAdd =
+        {
+            .type = protocolClientLocal,
+            .storageType = protocolStorageTypePg,
+        };
+
+        lstInsert(protocolHelper.clientList, 0, &protocolHelperClientAdd);
+
         TEST_RESULT_VOID(protocolKeepAlive(), "keep alive");
+        lstRemoveIdx(protocolHelper.clientList, 0);
+
         TEST_RESULT_VOID(protocolFree(), "free remote protocol objects");
         TEST_RESULT_VOID(protocolFree(), "free remote protocol objects again");
 
@@ -1329,11 +1340,22 @@ testRun(void)
         HRN_CFG_LOAD(cfgCmdArchiveGet, argList, .role = cfgCmdRoleLocal);
 
         TEST_RESULT_STR_Z(cfgOptionStr(cfgOptRepoCipherPass), "acbd", "check cipher pass before");
-        TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 0), "get remote protocol");
+        TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 0, true), "get remote protocol");
-        TEST_RESULT_PTR(protocolHelper.clientRemote[0].client, client, "check position in cache");
         TEST_RESULT_STR_Z(cfgOptionStr(cfgOptRepoCipherPass), "acbd", "check cipher pass after");
 
-        TEST_RESULT_VOID(protocolFree(), "free remote protocol objects");
+        // Remove the client from the client list so it is not found
+        ProtocolHelperClient clientHelper = *(ProtocolHelperClient *)lstGet(protocolHelper.clientList, 0);
+        lstRemoveIdx(protocolHelper.clientList, 0);
+
+        TEST_RESULT_VOID(protocolHelperFree(client), "free missing remote protocol object");
+
+        // Add client back so it can be removed -- also add a fake client that will be skipped
+        lstAdd(protocolHelper.clientList, &(ProtocolHelperClient){0});
+        lstAdd(protocolHelper.clientList, &clientHelper);
+
+        TEST_RESULT_VOID(protocolHelperFree(client), "free remote protocol object");
+
+        lstRemoveIdx(protocolHelper.clientList, 0);
 
         // -------------------------------------------------------------------------------------------------------------------------
         TEST_TITLE("start protocol with remote encryption settings");
@@ -1362,11 +1384,11 @@ testRun(void)
         HRN_CFG_LOAD(cfgCmdCheck, argList);
 
         TEST_RESULT_PTR(cfgOptionIdxStrNull(cfgOptRepoCipherPass, 0), NULL, "check repo1 cipher pass before");
-        TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 0), "get repo1 remote protocol");
+        TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 0, true), "get repo1 remote protocol");
         TEST_RESULT_STR_Z(cfgOptionIdxStr(cfgOptRepoCipherPass, 0), "dcba", "check repo1 cipher pass after");
 
         TEST_RESULT_PTR(cfgOptionIdxStrNull(cfgOptRepoCipherPass, 1), NULL, "check repo2 cipher pass before");
-        TEST_RESULT_VOID(protocolRemoteGet(protocolStorageTypeRepo, 1), "get repo2 remote protocol");
+        TEST_RESULT_VOID(protocolRemoteGet(protocolStorageTypeRepo, 1, true), "get repo2 remote protocol");
         TEST_RESULT_STR_Z(cfgOptionIdxStr(cfgOptRepoCipherPass, 1), "xxxx", "check repo2 cipher pass after");
 
         TEST_RESULT_VOID(protocolFree(), "free remote protocol objects");
@@ -1383,7 +1405,7 @@ testRun(void)
         hrnCfgArgKeyRawZ(argList, cfgOptPgPath, 1, TEST_PATH);
         HRN_CFG_LOAD(cfgCmdBackup, argList);
 
-        TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypePg, 0), "get remote protocol");
+        TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypePg, 0, true), "get remote protocol");
 
         TEST_RESULT_VOID(protocolFree(), "free local and remote protocol objects");
 
@@ -1398,7 +1420,6 @@ testRun(void)
 
         TEST_ASSIGN(client, protocolLocalGet(protocolStorageTypeRepo, 0, 1), "get local protocol");
         TEST_RESULT_PTR(protocolLocalGet(protocolStorageTypeRepo, 0, 1), client, "get local cached protocol");
-        TEST_RESULT_PTR(protocolHelper.clientLocal[0].client, client, "check location in cache");
 
         TEST_RESULT_VOID(protocolFree(), "free local and remote protocol objects");
     }

test/src/module/storage/remoteTest.c

@@ -784,7 +784,7 @@ testRun(void)
     // sure coverage data has been written by the remote. We also need to make sure that the mem context callback is cleared so that
     // protocolClientFreeResource() will not be called and send another exit. protocolFree() is still required to free the client
     // objects.
-    ProtocolClient *client = protocolRemoteGet(protocolStorageTypeRepo, 0);
+    ProtocolClient *client = protocolRemoteGet(protocolStorageTypeRepo, 0, true);
 
     memContextCallbackClear(objMemContext(client));
 
@@ -793,7 +793,7 @@ testRun(void)
 
     protocolClientRequestP(client, PROTOCOL_COMMAND_EXIT);
 
-    client = protocolRemoteGet(protocolStorageTypePg, 1);
+    client = protocolRemoteGet(protocolStorageTypePg, 1, true);
 
     for (unsigned int sessionIdx = 0; sessionIdx < lstSize(client->sessionList); sessionIdx++)
         memContextCallbackClear(objMemContext(*(ProtocolClientSession **)lstGet(client->sessionList, sessionIdx)));