1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-07-15 01:04:37 +02:00

Fix leaks in protocol module.

These leaks were not a big deal individually and there are generally few protocol objects created, but the leaks ended up in mem contexts that persist for most of execution. It makes sense to keep long-lived contexts as tidy as possible.
This commit is contained in:
David Steele
2022-04-26 11:59:21 -04:00
parent 78e912a932
commit a56fa0eb45

View File

@ -171,7 +171,11 @@ protocolLocalParam(ProtocolStorageType protocolStorageType, unsigned int hostIdx
// Disable output to stdout since it is used by the protocol // Disable output to stdout since it is used by the protocol
kvPut(optionReplace, VARSTRDEF(CFGOPT_LOG_LEVEL_CONSOLE), VARSTRDEF("off")); kvPut(optionReplace, VARSTRDEF(CFGOPT_LOG_LEVEL_CONSOLE), VARSTRDEF("off"));
result = strLstMove(cfgExecParam(cfgCommand(), cfgCmdRoleLocal, optionReplace, true, false), memContextPrior()); MEM_CONTEXT_PRIOR_BEGIN()
{
result = cfgExecParam(cfgCommand(), cfgCmdRoleLocal, optionReplace, true, false);
}
MEM_CONTEXT_PRIOR_END();
} }
MEM_CONTEXT_TEMP_END(); MEM_CONTEXT_TEMP_END();
@ -193,19 +197,34 @@ protocolLocalExec(
ASSERT(helper != NULL); ASSERT(helper != NULL);
MEM_CONTEXT_TEMP_BEGIN()
{
// Execute the protocol command // Execute the protocol command
helper->exec = execNew( const String *name = strNewFmt(PROTOCOL_SERVICE_LOCAL "-%u process", processId);
cfgExe(), protocolLocalParam(protocolStorageType, hostIdx, processId), const StringList *const param = protocolLocalParam(protocolStorageType, hostIdx, processId);
strNewFmt(PROTOCOL_SERVICE_LOCAL "-%u process", processId), cfgOptionUInt64(cfgOptProtocolTimeout));
MEM_CONTEXT_PRIOR_BEGIN()
{
helper->exec = execNew(cfgExe(), param, name, cfgOptionUInt64(cfgOptProtocolTimeout));
}
MEM_CONTEXT_PRIOR_END();
execOpen(helper->exec); execOpen(helper->exec);
// Create protocol object // Create protocol object
name = strNewFmt(PROTOCOL_SERVICE_LOCAL "-%u protocol", processId);
MEM_CONTEXT_PRIOR_BEGIN()
{
helper->client = protocolClientNew( helper->client = protocolClientNew(
strNewFmt(PROTOCOL_SERVICE_LOCAL "-%u protocol", processId), name, PROTOCOL_SERVICE_LOCAL_STR, execIoRead(helper->exec), execIoWrite(helper->exec));
PROTOCOL_SERVICE_LOCAL_STR, execIoRead(helper->exec), execIoWrite(helper->exec)); }
MEM_CONTEXT_PRIOR_END();
// Move client to exec context so they are freed together // Move client to exec context so they are freed together
protocolClientMove(helper->client, execMemContext(helper->exec)); protocolClientMove(helper->client, execMemContext(helper->exec));
}
MEM_CONTEXT_TEMP_END();
FUNCTION_TEST_RETURN_VOID(); FUNCTION_TEST_RETURN_VOID();
} }
@ -332,6 +351,10 @@ protocolServerAuthorize(const String *authListStr, const String *const stanza)
ASSERT(authListStr != NULL); ASSERT(authListStr != NULL);
bool result = false;
MEM_CONTEXT_TEMP_BEGIN()
{
// Empty list is not valid. ??? It would be better if this were done during config parsing. // Empty list is not valid. ??? It would be better if this were done during config parsing.
authListStr = strTrim(strDup(authListStr)); authListStr = strTrim(strDup(authListStr));
@ -340,21 +363,27 @@ protocolServerAuthorize(const String *authListStr, const String *const stanza)
// If * then all stanzas are authorized // If * then all stanzas are authorized
if (strEqZ(authListStr, "*")) if (strEqZ(authListStr, "*"))
FUNCTION_TEST_RETURN(BOOL, true); {
result = true;
// Check the list of stanzas for a match with the specified stanza. Each entry will need to be trimmed before comparing. }
if (stanza != NULL) // Else check the stanza list for a match with the specified stanza. Each entry will need to be trimmed before comparing.
else if (stanza != NULL)
{ {
StringList *authList = strLstNewSplitZ(authListStr, COMMA_Z); StringList *authList = strLstNewSplitZ(authListStr, COMMA_Z);
for (unsigned int authListIdx = 0; authListIdx < strLstSize(authList); authListIdx++) for (unsigned int authListIdx = 0; authListIdx < strLstSize(authList); authListIdx++)
{ {
if (strEq(strTrim(strLstGet(authList, authListIdx)), stanza)) if (strEq(strTrim(strLstGet(authList, authListIdx)), stanza))
FUNCTION_TEST_RETURN(BOOL, true); {
result = true;
break;
} }
} }
}
}
MEM_CONTEXT_TEMP_END();
FUNCTION_TEST_RETURN(BOOL, false); FUNCTION_TEST_RETURN(BOOL, result);
} }
ProtocolServer * ProtocolServer *
@ -443,6 +472,10 @@ protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int hostId
FUNCTION_LOG_PARAM(UINT, hostIdx); FUNCTION_LOG_PARAM(UINT, hostIdx);
FUNCTION_LOG_END(); FUNCTION_LOG_END();
StringList *result = NULL;
MEM_CONTEXT_TEMP_BEGIN()
{
// Is this a repo remote? // Is this a repo remote?
bool isRepo = protocolStorageType == protocolStorageTypeRepo; bool isRepo = protocolStorageType == protocolStorageTypeRepo;
@ -563,7 +596,15 @@ protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int hostId
// Add the remote type // Add the remote type
kvPut(optionReplace, VARSTRDEF(CFGOPT_REMOTE_TYPE), VARSTR(strIdToStr(protocolStorageType))); kvPut(optionReplace, VARSTRDEF(CFGOPT_REMOTE_TYPE), VARSTR(strIdToStr(protocolStorageType)));
FUNCTION_LOG_RETURN(STRING_LIST, cfgExecParam(cfgCommand(), cfgCmdRoleRemote, optionReplace, false, true)); MEM_CONTEXT_PRIOR_BEGIN()
{
result = cfgExecParam(cfgCommand(), cfgCmdRoleRemote, optionReplace, false, true);
}
MEM_CONTEXT_PRIOR_END();
}
MEM_CONTEXT_TEMP_END();
FUNCTION_LOG_RETURN(STRING_LIST, result);
} }
// Helper to add SSH parameters when executing the remote via SSH // Helper to add SSH parameters when executing the remote via SSH
@ -575,7 +616,7 @@ protocolRemoteParamSsh(const ProtocolStorageType protocolStorageType, const unsi
FUNCTION_LOG_PARAM(UINT, hostIdx); FUNCTION_LOG_PARAM(UINT, hostIdx);
FUNCTION_LOG_END(); FUNCTION_LOG_END();
StringList *result = NULL; StringList *result = strLstNew();
MEM_CONTEXT_TEMP_BEGIN() MEM_CONTEXT_TEMP_BEGIN()
{ {
@ -583,7 +624,6 @@ protocolRemoteParamSsh(const ProtocolStorageType protocolStorageType, const unsi
bool isRepo = protocolStorageType == protocolStorageTypeRepo; bool isRepo = protocolStorageType == protocolStorageTypeRepo;
// Fixed parameters for ssh command // Fixed parameters for ssh command
result = strLstNew();
strLstAddZ(result, "-o"); strLstAddZ(result, "-o");
strLstAddZ(result, "LogLevel=error"); strLstAddZ(result, "LogLevel=error");
strLstAddZ(result, "-o"); strLstAddZ(result, "-o");
@ -610,9 +650,6 @@ protocolRemoteParamSsh(const ProtocolStorageType protocolStorageType, const unsi
strLstInsert(paramList, 0, cfgOptionIdxStr(isRepo ? cfgOptRepoHostCmd : cfgOptPgHostCmd, hostIdx)); strLstInsert(paramList, 0, cfgOptionIdxStr(isRepo ? cfgOptRepoHostCmd : cfgOptPgHostCmd, hostIdx));
strLstAdd(result, strLstJoin(paramList, " ")); strLstAdd(result, strLstJoin(paramList, " "));
// Move to prior context
strLstMove(result, memContextPrior());
} }
MEM_CONTEXT_TEMP_END(); MEM_CONTEXT_TEMP_END();
@ -635,6 +672,8 @@ protocolRemoteExec(
ASSERT(helper != NULL); ASSERT(helper != NULL);
MEM_CONTEXT_TEMP_BEGIN()
{
// Get remote info // Get remote info
const bool isRepo = protocolStorageType == protocolStorageTypeRepo; const bool isRepo = protocolStorageType == protocolStorageTypeRepo;
const StringId remoteType = cfgOptionIdxStrId(isRepo ? cfgOptRepoHostType : cfgOptPgHostType, hostIdx); const StringId remoteType = cfgOptionIdxStrId(isRepo ? cfgOptRepoHostType : cfgOptPgHostType, hostIdx);
@ -650,10 +689,15 @@ protocolRemoteExec(
case CFGOPTVAL_REPO_HOST_TYPE_SSH: case CFGOPTVAL_REPO_HOST_TYPE_SSH:
{ {
// Exec SSH // Exec SSH
helper->exec = execNew( const String *const name = strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u process on '%s'", processId, strZ(host));
cfgOptionStr(cfgOptCmdSsh), protocolRemoteParamSsh(protocolStorageType, hostIdx), const StringList *const param = protocolRemoteParamSsh(protocolStorageType, hostIdx);
strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u process on '%s'", processId, strZ(host)),
cfgOptionUInt64(cfgOptProtocolTimeout)); MEM_CONTEXT_PRIOR_BEGIN()
{
helper->exec = execNew(cfgOptionStr(cfgOptCmdSsh), param, name, cfgOptionUInt64(cfgOptProtocolTimeout));
}
MEM_CONTEXT_PRIOR_END();
execOpen(helper->exec); execOpen(helper->exec);
read = execIoRead(helper->exec); read = execIoRead(helper->exec);
@ -668,6 +712,8 @@ protocolRemoteExec(
ASSERT(remoteType == CFGOPTVAL_REPO_HOST_TYPE_TLS); ASSERT(remoteType == CFGOPTVAL_REPO_HOST_TYPE_TLS);
// Negotiate TLS // Negotiate TLS
MEM_CONTEXT_PRIOR_BEGIN()
{
helper->ioClient = tlsClientNewP( helper->ioClient = tlsClientNewP(
sckClientNew( sckClientNew(
host, cfgOptionIdxUInt(isRepo ? cfgOptRepoHostPort : cfgOptPgHostPort, hostIdx), host, cfgOptionIdxUInt(isRepo ? cfgOptRepoHostPort : cfgOptPgHostPort, hostIdx),
@ -677,7 +723,10 @@ protocolRemoteExec(
.caPath = cfgOptionIdxStrNull(isRepo ? cfgOptRepoHostCaPath : cfgOptPgHostCaPath, hostIdx), .caPath = cfgOptionIdxStrNull(isRepo ? cfgOptRepoHostCaPath : cfgOptPgHostCaPath, hostIdx),
.certFile = cfgOptionIdxStr(isRepo ? cfgOptRepoHostCertFile : cfgOptPgHostCertFile, hostIdx), .certFile = cfgOptionIdxStr(isRepo ? cfgOptRepoHostCertFile : cfgOptPgHostCertFile, hostIdx),
.keyFile = cfgOptionIdxStr(isRepo ? cfgOptRepoHostKeyFile : cfgOptPgHostKeyFile, hostIdx)); .keyFile = cfgOptionIdxStr(isRepo ? cfgOptRepoHostKeyFile : cfgOptPgHostKeyFile, hostIdx));
helper->ioSession = ioClientOpen(helper->ioClient); helper->ioSession = ioClientOpen(helper->ioClient);
}
MEM_CONTEXT_PRIOR_END();
read = ioSessionIoReadP(helper->ioSession); read = ioSessionIoReadP(helper->ioSession);
write = ioSessionIoWrite(helper->ioSession); write = ioSessionIoWrite(helper->ioSession);
@ -687,9 +736,14 @@ protocolRemoteExec(
} }
// Create protocol object // Create protocol object
helper->client = protocolClientNew( const String *const name = strNewFmt(
strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u %s protocol on '%s'", processId, strZ(strIdToStr(remoteType)), strZ(host)), PROTOCOL_SERVICE_REMOTE "-%u %s protocol on '%s'", processId, strZ(strIdToStr(remoteType)), strZ(host));
PROTOCOL_SERVICE_REMOTE_STR, read, write);
MEM_CONTEXT_PRIOR_BEGIN()
{
helper->client = protocolClientNew(name, PROTOCOL_SERVICE_REMOTE_STR, read, write);
}
MEM_CONTEXT_PRIOR_END();
// Remote initialization // Remote initialization
switch (remoteType) switch (remoteType)
@ -706,13 +760,16 @@ protocolRemoteExec(
ASSERT(remoteType == CFGOPTVAL_REPO_HOST_TYPE_TLS); ASSERT(remoteType == CFGOPTVAL_REPO_HOST_TYPE_TLS);
// Pass parameters to server // Pass parameters to server
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_CONFIG); ProtocolCommand *const command = protocolCommandNew(PROTOCOL_COMMAND_CONFIG);
pckWriteStrLstP(protocolCommandParam(command), protocolRemoteParam(protocolStorageType, hostIdx)); pckWriteStrLstP(protocolCommandParam(command), protocolRemoteParam(protocolStorageType, hostIdx));
protocolClientExecute(helper->client, command, false); protocolClientExecute(helper->client, command, false);
protocolCommandFree(command);
break; break;
} }
} }
}
MEM_CONTEXT_TEMP_END();
FUNCTION_TEST_RETURN_VOID(); FUNCTION_TEST_RETURN_VOID();
} }