db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-01-18 04:58:51 +02:00
Code Issues Releases Activity

Fix issue setting log-level-file=off for the archive-get command.

Browse Source 
This problem was not specific to archive-get, but that was the only place it was expressing in the last release.  The new archive-push was also affected.

The issue was with daemon processes that had closed all their file descriptors.  When exec'ing and setting up pipes to communicate with a child process the dup2() function created file descriptors that overlapped with the first descriptor (stdout) that was being duped into.  This descriptor was subsequently closed and wackiness ensued.

If logging was enabled (the default) that increased all the file descriptors by one and everything worked.

Fix this by checking if the file descriptor to be closed is the same one being dup'd into.  This solution may not be generally applicable but it works fine in this case.

Reported by Brad Nicholson.
This commit is contained in:
David Steele 2019-04-08 17:21:20 -04:00
parent 8ac422dca9
commit 21c83eea59
3 changed files with 61 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
doc/xml/release.xml
View File

@ -23,6 +23,14 @@
<p>Fix issues when a path option is / terminated.</p>
</release-item>
<release-item>
<release-item-contributor-list>
<release-item-ideator id="brad.nicholson"/>
</release-item-contributor-list>
<p>Fix issue setting <br-option>log-level-file=off</br-option> for the <cmd>archive-get</cmd> command.</p>
</release-item>
<release-item>
<p>Fix issues with <code>remote</code>/<code>local</code> command logging options.</p>
</release-item>

38
src/common/exec.c
View File

@ -42,6 +42,29 @@ struct Exec
IoWrite *ioWriteExec; // Wrapper for handle write interface
};
/***********************************************************************************************************************************
Macro to close file descriptors after dup2() in the child process
If the parent process is daemomized and has closed stdout, stdin, and stderr or some combination of them, then the newly created
descriptors might overlap stdout, stdin, or stderr. In that case we don't want to accidentally close the descriptor that we have
just copied.
Note that this is pretty specific to the way that file descriptors are handled in this module and may not be generally applicable in
other code.
***********************************************************************************************************************************/
#define PIPE_DUP2(pipe, pipeIdx, fd) \
do \
{ \
dup2(pipe[pipeIdx], fd); \
\
if (pipe[0] != fd) \
close(pipe[0]); \
\
if (pipe[1] != fd) \
close(pipe[1]); \
} \
while (0);
/***********************************************************************************************************************************
New object
***********************************************************************************************************************************/
@ -116,17 +139,14 @@ execOpen(Exec *this)
// Disable logging and close log file
logClose();
// Assign stdout to the input side of the read pipe and close the unused handle
dup2(pipeRead[1], STDOUT_FILENO);
close(pipeRead[0]);
// Assign stdout to the input side of the read pipe
PIPE_DUP2(pipeRead, 1, STDOUT_FILENO);
// Assign stdin to the output side of the write pipe and close the unused handle
dup2(pipeWrite[0], STDIN_FILENO);
close(pipeWrite[1]);
// Assign stdin to the output side of the write pipe
PIPE_DUP2(pipeWrite, 0, STDIN_FILENO);
// Assign stderr to the input side of the error pipe and close the unused handle
dup2(pipeError[1], STDERR_FILENO);
close(pipeError[0]);
// Assign stderr to the input side of the error pipe
PIPE_DUP2(pipeError, 1, STDERR_FILENO);
// Execute the binary. This statement will not return if it is successful
execvp(strPtr(this->command), (char ** const)strLstPtr(this->param));

24
test/src/module/common/execTest.c
View File

@ -1,6 +1,7 @@
/***********************************************************************************************************************************
Execute Process
***********************************************************************************************************************************/
#include "common/harnessFork.h"
/***********************************************************************************************************************************
Test Run
@ -61,6 +62,29 @@ testRun(void)
TEST_RESULT_STR(strPtr(ioReadLine(execIoRead(exec))), " 1\tACKBYACK", "read cat exec");
TEST_RESULT_VOID(execFree(exec), "free exec");
// Run the same test as above but close all file descriptors first to ensure we don't accidentally close a required
// descriptor while running dup2()/close() between the fork() and the exec().
// -------------------------------------------------------------------------------------------------------------------------
HARNESS_FORK_BEGIN()
{
HARNESS_FORK_CHILD_BEGIN(0, false)
{
// This is not really fd max but for the purposes of testing is fine -- we won't have more than 64 fds open
for (int fd = 0; fd < 64; fd++)
close(fd);
TEST_ASSIGN(exec, execNew(strNew("cat"), strLstAddZ(strLstNew(), "-b"), strNew("cat"), 1000), "new cat exec");
TEST_RESULT_VOID(execOpen(exec), "open cat exec");
TEST_RESULT_VOID(ioWriteLine(execIoWrite(exec), message), "write cat exec");
ioWriteFlush(execIoWrite(exec));
TEST_RESULT_STR(strPtr(ioReadLine(execIoRead(exec))), " 1\tACKBYACK", "read cat exec");
TEST_RESULT_VOID(execFree(exec), "free exec");
}
HARNESS_FORK_CHILD_END();
}
HARNESS_FORK_END();
// -------------------------------------------------------------------------------------------------------------------------
TEST_ASSIGN(exec, execNew(strNew("sleep"), strLstAddZ(strLstNew(), "2"), strNew("sleep"), 1000), "new sleep exec");
TEST_RESULT_VOID(execOpen(exec), "open cat exec");