From 1ea34407b83ef9fbf21c4f8389f85377d8eee619 Mon Sep 17 00:00:00 2001 From: Thomas Faber Date: Mon, 16 Feb 2015 13:17:04 +0000 Subject: [PATCH] [ROSAUTOTEST] - Abstract unidirectional anonymous pipes into a CPipe class - Abstract a process with redirected output into a CPipedProcess class - Use these abstractions to avoid polling for output from test processes. Instead, use blocking read operations to yield the CPU while waiting for data. ROSTESTS-144 #resolve svn path=/trunk/; revision=66316 --- rostests/rosautotest/CMakeLists.txt | 2 + rostests/rosautotest/CPipe.cpp | 143 +++++++++++++++++++++++++ rostests/rosautotest/CPipe.h | 26 +++++ rostests/rosautotest/CPipedProcess.cpp | 42 ++++++++ rostests/rosautotest/CPipedProcess.h | 17 +++ rostests/rosautotest/CWineTest.cpp | 77 +++---------- rostests/rosautotest/CWineTest.h | 3 - rostests/rosautotest/precomp.h | 2 + 8 files changed, 249 insertions(+), 63 deletions(-) create mode 100644 rostests/rosautotest/CPipe.cpp create mode 100644 rostests/rosautotest/CPipe.h create mode 100644 rostests/rosautotest/CPipedProcess.cpp create mode 100644 rostests/rosautotest/CPipedProcess.h diff --git a/rostests/rosautotest/CMakeLists.txt b/rostests/rosautotest/CMakeLists.txt index b48962e9960..18af31412a1 100644 --- a/rostests/rosautotest/CMakeLists.txt +++ b/rostests/rosautotest/CMakeLists.txt @@ -6,6 +6,8 @@ list(APPEND SOURCE CFatalException.cpp CInvalidParameterException.cpp CJournaledTestList.cpp + CPipe.cpp + CPipedProcess.cpp CProcess.cpp CSimpleException.cpp CTest.cpp diff --git a/rostests/rosautotest/CPipe.cpp b/rostests/rosautotest/CPipe.cpp new file mode 100644 index 00000000000..8d87723858d --- /dev/null +++ b/rostests/rosautotest/CPipe.cpp @@ -0,0 +1,143 @@ +/* + * PROJECT: ReactOS Automatic Testing Utility + * LICENSE: GNU GPLv2 or any later version as published by the Free Software Foundation + * PURPOSE: Class that managed an unidirectional anonymous byte stream pipe + * COPYRIGHT: Copyright 2015 Thomas Faber + */ + +#include "precomp.h" + +/** + * Constructs a CPipe object and initializes read and write handles. + */ +CPipe::CPipe() +{ + SECURITY_ATTRIBUTES SecurityAttributes; + + SecurityAttributes.nLength = sizeof(SecurityAttributes); + SecurityAttributes.bInheritHandle = TRUE; + SecurityAttributes.lpSecurityDescriptor = NULL; + + if(!CreatePipe(&m_hReadPipe, &m_hWritePipe, &SecurityAttributes, 0)) + FATAL("CreatePipe failed\n"); +} + +/** + * Destructs a CPipe object and closes all open handles. + */ +CPipe::~CPipe() +{ + if (m_hReadPipe) + CloseHandle(m_hReadPipe); + if (m_hWritePipe) + CloseHandle(m_hWritePipe); +} + +/** + * Closes a CPipe's read pipe, leaving the write pipe unchanged. + */ +void +CPipe::CloseReadPipe() +{ + if (!m_hReadPipe) + FATAL("Trying to close already closed read pipe"); + CloseHandle(m_hReadPipe); +} + +/** + * Closes a CPipe's write pipe, leaving the read pipe unchanged. + */ +void +CPipe::CloseWritePipe() +{ + if (!m_hWritePipe) + FATAL("Trying to close already closed write pipe"); + CloseHandle(m_hWritePipe); +} + +/** + * Reads data from a pipe without advancing the read offset and/or retrieves information about available data. + * + * This function must not be called after CloseReadPipe. + * + * @param Buffer + * An optional buffer to read pipe data into. + * + * @param BufferSize + * The size of the buffer specified in Buffer, or 0 if no read should be performed. + * + * @param BytesRead + * On return, the number of bytes actually read from the pipe into Buffer. + * + * @param TotalBytesAvailable + * On return, the total number of bytes available to read from the pipe. + * + * @return + * True on success, false on failure; call GetLastError for error information. + * + * @see PeekNamedPipe + */ +bool +CPipe::Peek(PVOID Buffer, DWORD BufferSize, PDWORD BytesRead, PDWORD TotalBytesAvailable) +{ + if (!m_hReadPipe) + FATAL("Trying to peek from a closed read pipe"); + + return PeekNamedPipe(m_hReadPipe, Buffer, BufferSize, BytesRead, TotalBytesAvailable, NULL); +} + +/** + * Reads data from the read pipe, advancing the read offset accordingly. + * + * This function must not be called after CloseReadPipe. + * + * @param Buffer + * Buffer to read pipe data into. + * + * @param NumberOfBytesToRead + * The number of bytes to read into Buffer. + * + * @param NumberOfBytesRead + * On return, the number of bytes actually read from the pipe into Buffer. + * + * @return + * True on success, false on failure; call GetLastError for error information. + * + * @see ReadFile + */ +bool +CPipe::Read(PVOID Buffer, DWORD NumberOfBytesToRead, PDWORD NumberOfBytesRead) +{ + if (!m_hReadPipe) + FATAL("Trying to read from a closed read pipe"); + + return ReadFile(m_hReadPipe, Buffer, NumberOfBytesToRead, NumberOfBytesRead, NULL); +} + +/** + * Writes data to the write pipe. + * + * This function must not be called after CloseWritePipe. + * + * @param Buffer + * Buffer containing the data to write. + * + * @param NumberOfBytesToWrite + * The number of bytes to write to the pipe from Buffer. + * + * @param NumberOfBytesWritten + * On return, the number of bytes actually written to the pipe. + * + * @return + * True on success, false on failure; call GetLastError for error information. + * + * @see WriteFile + */ +bool +CPipe::Write(LPCVOID Buffer, DWORD NumberOfBytesToWrite, PDWORD NumberOfBytesWritten) +{ + if (!m_hWritePipe) + FATAL("Trying to write to a closed write pipe"); + + return WriteFile(m_hWritePipe, Buffer, NumberOfBytesToWrite, NumberOfBytesWritten, NULL); +} diff --git a/rostests/rosautotest/CPipe.h b/rostests/rosautotest/CPipe.h new file mode 100644 index 00000000000..7049dc039d1 --- /dev/null +++ b/rostests/rosautotest/CPipe.h @@ -0,0 +1,26 @@ +/* + * PROJECT: ReactOS Automatic Testing Utility + * LICENSE: GNU GPLv2 or any later version as published by the Free Software Foundation + * PURPOSE: Class that managed an unidirectional anonymous byte stream pipe + * COPYRIGHT: Copyright 2015 Thomas Faber + */ + +class CPipe +{ +private: + HANDLE m_hReadPipe; + HANDLE m_hWritePipe; + +public: + CPipe(); + ~CPipe(); + + void CloseReadPipe(); + void CloseWritePipe(); + + bool Peek(PVOID Buffer, DWORD BufferSize, PDWORD BytesRead, PDWORD TotalBytesAvailable); + bool Read(PVOID Buffer, DWORD NumberOfBytesToRead, PDWORD NumberOfBytesRead); + bool Write(LPCVOID Buffer, DWORD NumberOfBytesToWrite, PDWORD NumberOfBytesWritten); + + friend class CPipedProcess; +}; diff --git a/rostests/rosautotest/CPipedProcess.cpp b/rostests/rosautotest/CPipedProcess.cpp new file mode 100644 index 00000000000..896501f44ce --- /dev/null +++ b/rostests/rosautotest/CPipedProcess.cpp @@ -0,0 +1,42 @@ +/* + * PROJECT: ReactOS Automatic Testing Utility + * LICENSE: GNU GPLv2 or any later version as published by the Free Software Foundation + * PURPOSE: Class that creates a process and redirects its output to a pipe + * COPYRIGHT: Copyright 2015 Thomas Faber + */ + +#include "precomp.h" + +/** + * Constructs a CPipedProcess object and starts the process with redirected output. + * + * @param CommandLine + * A std::wstring containing the command line to run. + * + * @param Pipe + * The CPipe instance to redirect the process's output to. + * Note that only the read pipe is usable after the pipe was passed to this object. + */ +CPipedProcess::CPipedProcess(const wstring& CommandLine, CPipe& Pipe) + : CProcess(CommandLine, InitStartupInfo(Pipe)) +{ + Pipe.CloseWritePipe(); +} + +/** + * Initializes the STARTUPINFO structure for use in CreateProcessW. + * + * @param Pipe + * The CPipe instance to redirect the process's output to. + */ +LPSTARTUPINFOW +CPipedProcess::InitStartupInfo(CPipe& Pipe) +{ + ZeroMemory(&m_StartupInfo, sizeof(m_StartupInfo)); + m_StartupInfo.cb = sizeof(m_StartupInfo); + m_StartupInfo.dwFlags = STARTF_USESTDHANDLES; + m_StartupInfo.hStdInput = GetStdHandle(STD_INPUT_HANDLE); + m_StartupInfo.hStdOutput = Pipe.m_hWritePipe; + m_StartupInfo.hStdError = Pipe.m_hWritePipe; + return &m_StartupInfo; +} diff --git a/rostests/rosautotest/CPipedProcess.h b/rostests/rosautotest/CPipedProcess.h new file mode 100644 index 00000000000..f279cc5db79 --- /dev/null +++ b/rostests/rosautotest/CPipedProcess.h @@ -0,0 +1,17 @@ +/* + * PROJECT: ReactOS Automatic Testing Utility + * LICENSE: GNU GPLv2 or any later version as published by the Free Software Foundation + * PURPOSE: Class that creates a process and redirects its output to a pipe + * COPYRIGHT: Copyright 2015 Thomas Faber + */ + +class CPipedProcess : public CProcess +{ +private: + STARTUPINFOW m_StartupInfo; + + LPSTARTUPINFOW InitStartupInfo(CPipe& Pipe); + +public: + CPipedProcess(const wstring& CommandLine, CPipe& Pipe); +}; diff --git a/rostests/rosautotest/CWineTest.cpp b/rostests/rosautotest/CWineTest.cpp index 16aaa1a8f89..7cf27821a42 100644 --- a/rostests/rosautotest/CWineTest.cpp +++ b/rostests/rosautotest/CWineTest.cpp @@ -18,10 +18,7 @@ CWineTest::CWineTest() /* Zero-initialize variables */ m_hFind = NULL; - m_hReadPipe = NULL; - m_hWritePipe = NULL; m_ListBuffer = NULL; - memset(&m_StartupInfo, 0, sizeof(m_StartupInfo)); /* Set up m_TestPath */ if(!GetWindowsDirectoryW(WindowsDirectory, MAX_PATH)) @@ -39,12 +36,6 @@ CWineTest::~CWineTest() if(m_hFind) FindClose(m_hFind); - if(m_hReadPipe) - CloseHandle(m_hReadPipe); - - if(m_hWritePipe) - CloseHandle(m_hWritePipe); - if(m_ListBuffer) delete m_ListBuffer; } @@ -111,6 +102,7 @@ CWineTest::DoListCommand() DWORD BytesAvailable; DWORD Temp; wstring CommandLine; + CPipe Pipe; /* Build the command line */ CommandLine = m_TestPath; @@ -119,7 +111,7 @@ CWineTest::DoListCommand() { /* Start the process for getting all available tests */ - CProcess Process(CommandLine, &m_StartupInfo); + CPipedProcess Process(CommandLine, Pipe); /* Wait till this process ended */ if(WaitForSingleObject(Process.GetProcessHandle(), ListTimeout) == WAIT_FAILED) @@ -127,8 +119,8 @@ CWineTest::DoListCommand() } /* Read the output data into a buffer */ - if(!PeekNamedPipe(m_hReadPipe, NULL, 0, NULL, &BytesAvailable, NULL)) - FATAL("PeekNamedPipe failed for the test list\n"); + if(!Pipe.Peek(NULL, 0, NULL, &BytesAvailable)) + FATAL("CPipe::Peek failed for the test list\n"); /* Check if we got any */ if(!BytesAvailable) @@ -142,8 +134,8 @@ CWineTest::DoListCommand() /* Read the data */ m_ListBuffer = new char[BytesAvailable]; - if(!ReadFile(m_hReadPipe, m_ListBuffer, BytesAvailable, &Temp, NULL)) - FATAL("ReadPipe failed\n"); + if(!Pipe.Read(m_ListBuffer, BytesAvailable, &Temp)) + FATAL("CPipe::Read failed\n"); return BytesAvailable; } @@ -260,13 +252,13 @@ CWineTest::GetNextTestInfo() void CWineTest::RunTest(CTestInfo* TestInfo) { - bool BreakLoop = false; DWORD BytesAvailable; - DWORD Temp; stringstream ss, ssFinish; DWORD StartTime = GetTickCount(); float TotalTime; string tailString; + CPipe Pipe; + char Buffer[1024]; ss << "Running Wine Test, Module: " << TestInfo->Module << ", Test: " << TestInfo->Test << endl; StringOut(ss.str()); @@ -275,40 +267,20 @@ CWineTest::RunTest(CTestInfo* TestInfo) { /* Execute the test */ - CProcess Process(TestInfo->CommandLine, &m_StartupInfo); + CPipedProcess Process(TestInfo->CommandLine, Pipe); /* Receive all the data from the pipe */ - do + while(Pipe.Read(Buffer, sizeof(Buffer) - 1, &BytesAvailable) && BytesAvailable) { - /* When the application finished, make sure that we peek the pipe one more time, so that we get all data. - If the following condition would be the while() condition, we might hit a race condition: - - We check for data with PeekNamedPipe -> no data available - - The application outputs its data and finishes - - WaitForSingleObject reports that the application has finished and we break the loop without receiving any data - */ - if(WaitForSingleObject(Process.GetProcessHandle(), 0) != WAIT_TIMEOUT) - BreakLoop = true; + /* Output text through StringOut, even while the test is still running */ + Buffer[BytesAvailable] = 0; + tailString = StringOut(tailString.append(string(Buffer)), false); - if(!PeekNamedPipe(m_hReadPipe, NULL, 0, NULL, &BytesAvailable, NULL)) - FATAL("PeekNamedPipe failed for the test run\n"); - - if(BytesAvailable) - { - /* There is data, so get it and output it */ - auto_array_ptr Buffer(new char[BytesAvailable + 1]); - - if(!ReadFile(m_hReadPipe, Buffer, BytesAvailable, &Temp, NULL)) - FATAL("ReadFile failed for the test run\n"); - - /* Output text through StringOut, even while the test is still running */ - Buffer[BytesAvailable] = 0; - tailString = StringOut(tailString.append(string(Buffer)), false); - - if(Configuration.DoSubmit()) - TestInfo->Log += Buffer; - } + if(Configuration.DoSubmit()) + TestInfo->Log += Buffer; } - while(!BreakLoop); + if(GetLastError() != ERROR_BROKEN_PIPE) + FATAL("CPipe::Read failed for the test run\n"); } /* Print what's left */ @@ -330,21 +302,6 @@ CWineTest::Run() auto_ptr TestList; auto_ptr WebService; CTestInfo* TestInfo; - SECURITY_ATTRIBUTES SecurityAttributes; - - /* Create a pipe for getting the output of the tests */ - SecurityAttributes.nLength = sizeof(SecurityAttributes); - SecurityAttributes.bInheritHandle = TRUE; - SecurityAttributes.lpSecurityDescriptor = NULL; - - if(!CreatePipe(&m_hReadPipe, &m_hWritePipe, &SecurityAttributes, 0)) - FATAL("CreatePipe failed\n"); - - m_StartupInfo.cb = sizeof(m_StartupInfo); - m_StartupInfo.dwFlags = STARTF_USESTDHANDLES; - m_StartupInfo.hStdInput = GetStdHandle(STD_INPUT_HANDLE); - m_StartupInfo.hStdOutput = m_hWritePipe; - m_StartupInfo.hStdError = m_hWritePipe; /* The virtual test list is of course faster, so it should be preferred over the journaled one. diff --git a/rostests/rosautotest/CWineTest.h b/rostests/rosautotest/CWineTest.h index 58ca5585e84..5993822f09a 100644 --- a/rostests/rosautotest/CWineTest.h +++ b/rostests/rosautotest/CWineTest.h @@ -9,10 +9,7 @@ class CWineTest : public CTest { private: HANDLE m_hFind; - HANDLE m_hReadPipe; - HANDLE m_hWritePipe; PCHAR m_ListBuffer; - STARTUPINFOW m_StartupInfo; string m_CurrentTest; wstring m_CurrentFile; wstring m_CurrentListCommand; diff --git a/rostests/rosautotest/precomp.h b/rostests/rosautotest/precomp.h index 21e60133a38..b42fd4166e4 100644 --- a/rostests/rosautotest/precomp.h +++ b/rostests/rosautotest/precomp.h @@ -30,7 +30,9 @@ using namespace std; #include "CConfiguration.h" #include "CFatalException.h" #include "CInvalidParameterException.h" +#include "CPipe.h" #include "CProcess.h" +#include "CPipedProcess.h" #include "CSimpleException.h" #include "CTestInfo.h" #include "CTest.h"