Check thread invalid handle and remove extra dup2 call. (#1008)

This commit is contained in:
Justin Kotalik 2018-07-02 17:18:05 -07:00 committed by GitHub
parent dfd75e939d
commit 9b7ee92028
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 27 additions and 5 deletions

View File

@ -13,7 +13,7 @@ PipeOutputManager::PipeOutputManager() :
m_dwStdErrReadTotal(0), m_dwStdErrReadTotal(0),
m_hErrReadPipe(INVALID_HANDLE_VALUE), m_hErrReadPipe(INVALID_HANDLE_VALUE),
m_hErrWritePipe(INVALID_HANDLE_VALUE), m_hErrWritePipe(INVALID_HANDLE_VALUE),
m_hErrThread(INVALID_HANDLE_VALUE), m_hErrThread(NULL),
m_fDisposed(FALSE) m_fDisposed(FALSE)
{ {
InitializeSRWLock(&m_srwLock); InitializeSRWLock(&m_srwLock);
@ -54,7 +54,6 @@ PipeOutputManager::StopOutputRedirection()
if (m_fdPreviousStdOut >= 0) if (m_fdPreviousStdOut >= 0)
{ {
_dup2(m_fdPreviousStdOut, _fileno(stdout));
LOG_IF_DUPFAIL(_dup2(m_fdPreviousStdOut, _fileno(stdout))); LOG_IF_DUPFAIL(_dup2(m_fdPreviousStdOut, _fileno(stdout)));
} }
else else
@ -79,7 +78,6 @@ PipeOutputManager::StopOutputRedirection()
// GetExitCodeThread returns 0 on failure; thread status code is invalid. // GetExitCodeThread returns 0 on failure; thread status code is invalid.
if (m_hErrThread != NULL && if (m_hErrThread != NULL &&
m_hErrThread != INVALID_HANDLE_VALUE &&
!LOG_LAST_ERROR_IF(GetExitCodeThread(m_hErrThread, &dwThreadStatus) == 0) && !LOG_LAST_ERROR_IF(GetExitCodeThread(m_hErrThread, &dwThreadStatus) == 0) &&
dwThreadStatus == STILL_ACTIVE) dwThreadStatus == STILL_ACTIVE)
{ {
@ -96,8 +94,12 @@ PipeOutputManager::StopOutputRedirection()
} }
} }
CloseHandle(m_hErrThread); if (m_hErrThread != NULL)
m_hErrThread = NULL; {
CloseHandle(m_hErrThread);
m_hErrThread = NULL;
}
if (m_hErrReadPipe != INVALID_HANDLE_VALUE) if (m_hErrReadPipe != INVALID_HANDLE_VALUE)
{ {

View File

@ -32,6 +32,26 @@ namespace PipeOutputManagerTests
pManager->NotifyStartupComplete(); pManager->NotifyStartupComplete();
}
TEST(PipeManagerOutputTest, SetInvalidHandlesForErrAndOut)
{
auto m_fdPreviousStdOut = _dup(_fileno(stdout));
auto m_fdPreviousStdErr = _dup(_fileno(stderr));
SetStdHandle(STD_ERROR_HANDLE, INVALID_HANDLE_VALUE);
SetStdHandle(STD_OUTPUT_HANDLE, INVALID_HANDLE_VALUE);
PCWSTR expected = L"test";
PipeOutputManager* pManager = new PipeOutputManager();
ASSERT_EQ(S_OK, pManager->Start());
pManager->NotifyStartupComplete();
_dup2(m_fdPreviousStdOut, _fileno(stdout));
_dup2(m_fdPreviousStdErr, _fileno(stderr));
// Test will fail if we didn't redirect stdout back to a file descriptor. // Test will fail if we didn't redirect stdout back to a file descriptor.
// This is because gtest relies on console output to know if a test succeeded or failed. // This is because gtest relies on console output to know if a test succeeded or failed.
// If the output still points to a file/pipe, the test (and all other tests after it) will fail. // If the output still points to a file/pipe, the test (and all other tests after it) will fail.