From: Colin Finck Date: Tue, 23 Apr 2019 07:17:05 +0000 (+0200) Subject: [ROSAUTOTEST] Implement a process activity timeout of 2 minutes. If there is no log... X-Git-Tag: 0.4.14-dev~1124 X-Git-Url: https://git.reactos.org/?p=reactos.git;a=commitdiff_plain;h=7dd4d2256b97b1f8a3a392060592a3d0f715fbd7 [ROSAUTOTEST] Implement a process activity timeout of 2 minutes. If there is no log output within 2 minutes, the test process is killed, and we continue with the next test. This is a rather graceful approach compared to sysreg2's 3 minute timeout before killing and restarting the entire VM. Since we added autochk for FAT filesystems, the filesystem is often "fixed" after a reset with the consequence that ReactOS doesn't boot up anymore. The sysreg2 restart code still remains for handling tests causing BSODs. --- diff --git a/modules/rostests/rosautotest/CPipe.cpp b/modules/rostests/rosautotest/CPipe.cpp index 8fd3cf03e15..5af71639715 100644 --- a/modules/rostests/rosautotest/CPipe.cpp +++ b/modules/rostests/rosautotest/CPipe.cpp @@ -3,10 +3,14 @@ * LICENSE: GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+) * PURPOSE: Class that manages an unidirectional anonymous byte stream pipe * COPYRIGHT: Copyright 2015 Thomas Faber (thomas.faber@reactos.org) + * Copyright 2019 Colin Finck (colin@reactos.org) */ #include "precomp.h" +LONG CPipe::m_lPipeCount = 0; + + /** * Constructs a CPipe object and initializes read and write handles. */ @@ -18,8 +22,50 @@ CPipe::CPipe() SecurityAttributes.bInheritHandle = TRUE; SecurityAttributes.lpSecurityDescriptor = NULL; - if(!CreatePipe(&m_hReadPipe, &m_hWritePipe, &SecurityAttributes, 0)) - FATAL("CreatePipe failed\n"); + // Construct a unique pipe name. + WCHAR wszPipeName[MAX_PATH]; + InterlockedIncrement(&m_lPipeCount); + swprintf(wszPipeName, L"\\\\.\\pipe\\TestmanPipe%ld", m_lPipeCount); + + // Create a named pipe with the default settings, but overlapped (asynchronous) operations. + // Latter feature is why we can't simply use CreatePipe. + const DWORD dwDefaultBufferSize = 4096; + const DWORD dwDefaultTimeoutMilliseconds = 120000; + + m_hReadPipe = CreateNamedPipeW(wszPipeName, + PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED, + PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, + 1, + dwDefaultBufferSize, + dwDefaultBufferSize, + dwDefaultTimeoutMilliseconds, + &SecurityAttributes); + if (m_hReadPipe == INVALID_HANDLE_VALUE) + { + FATAL("CreateNamedPipe failed\n"); + } + + // Use CreateFileW to get the write handle to the pipe. + // Writing is done synchronously, so no FILE_FLAG_OVERLAPPED here! + m_hWritePipe = CreateFileW(wszPipeName, + GENERIC_WRITE, + 0, + &SecurityAttributes, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + NULL); + if (m_hWritePipe == INVALID_HANDLE_VALUE) + { + FATAL("CreateFileW failed\n"); + } + + // Prepare the OVERLAPPED structure for reading. + ZeroMemory(&m_ReadOverlapped, sizeof(m_ReadOverlapped)); + m_ReadOverlapped.hEvent = CreateEventW(NULL, TRUE, TRUE, NULL); + if (!m_ReadOverlapped.hEvent) + { + FATAL("CreateEvent failed\n"); + } } /** @@ -103,17 +149,60 @@ CPipe::Peek(PVOID Buffer, DWORD BufferSize, PDWORD BytesRead, PDWORD TotalBytesA * 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. + * Returns a Win32 error code. Expected error codes include: + * - ERROR_SUCCESS: The read has completed successfully. + * - WAIT_TIMEOUT: The given timeout has elapsed before any data was read. + * - ERROR_BROKEN_PIPE: The other end of the pipe has been closed. * * @see ReadFile */ -bool -CPipe::Read(PVOID Buffer, DWORD NumberOfBytesToRead, PDWORD NumberOfBytesRead) +DWORD +CPipe::Read(PVOID Buffer, DWORD NumberOfBytesToRead, PDWORD NumberOfBytesRead, DWORD TimeoutMilliseconds) { if (!m_hReadPipe) + { FATAL("Trying to read from a closed read pipe"); + } - return ReadFile(m_hReadPipe, Buffer, NumberOfBytesToRead, NumberOfBytesRead, NULL); + if (ReadFile(m_hReadPipe, Buffer, NumberOfBytesToRead, NumberOfBytesRead, &m_ReadOverlapped)) + { + // The asynchronous read request could be satisfied immediately. + return ERROR_SUCCESS; + } + else if (GetLastError() == ERROR_IO_PENDING) + { + // The asynchronous read request could not be satisfied immediately, so wait for it with the given timeout. + DWORD dwWaitResult = WaitForSingleObject(m_ReadOverlapped.hEvent, TimeoutMilliseconds); + if (dwWaitResult == WAIT_OBJECT_0) + { + // Fill NumberOfBytesRead. + if (GetOverlappedResult(m_hReadPipe, &m_ReadOverlapped, NumberOfBytesRead, FALSE)) + { + // We successfully read NumberOfBytesRead bytes. + return ERROR_SUCCESS; + } + else if (GetLastError() == ERROR_BROKEN_PIPE) + { + // The other end of the pipe has been closed. + return ERROR_BROKEN_PIPE; + } + else + { + // An unexpected error. + FATAL("GetOverlappedResult failed\n"); + } + } + else + { + // This may be WAIT_TIMEOUT or an unexpected error. + return dwWaitResult; + } + } + else + { + // This may be ERROR_BROKEN_PIPE or an unexpected error. + return GetLastError(); + } } /** diff --git a/modules/rostests/rosautotest/CPipe.h b/modules/rostests/rosautotest/CPipe.h index 6efe6703738..84bb01897f0 100644 --- a/modules/rostests/rosautotest/CPipe.h +++ b/modules/rostests/rosautotest/CPipe.h @@ -3,11 +3,15 @@ * LICENSE: GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+) * PURPOSE: Class that manages an unidirectional anonymous byte stream pipe * COPYRIGHT: Copyright 2015 Thomas Faber (thomas.faber@reactos.org) + * Copyright 2019 Colin Finck (colin@reactos.org) */ class CPipe { private: + static LONG m_lPipeCount; + + OVERLAPPED m_ReadOverlapped; HANDLE m_hReadPipe; HANDLE m_hWritePipe; @@ -19,7 +23,7 @@ public: void CloseWritePipe(); bool Peek(PVOID Buffer, DWORD BufferSize, PDWORD BytesRead, PDWORD TotalBytesAvailable); - bool Read(PVOID Buffer, DWORD NumberOfBytesToRead, PDWORD NumberOfBytesRead); + DWORD Read(PVOID Buffer, DWORD NumberOfBytesToRead, PDWORD NumberOfBytesRead, DWORD TimeoutMilliseconds); bool Write(LPCVOID Buffer, DWORD NumberOfBytesToWrite, PDWORD NumberOfBytesWritten); friend class CPipedProcess; diff --git a/modules/rostests/rosautotest/CProcess.cpp b/modules/rostests/rosautotest/CProcess.cpp index 65445cb379c..e462456564c 100644 --- a/modules/rostests/rosautotest/CProcess.cpp +++ b/modules/rostests/rosautotest/CProcess.cpp @@ -2,7 +2,7 @@ * PROJECT: ReactOS Automatic Testing Utility * LICENSE: GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+) * PURPOSE: Class able to create a new process and closing its handles on destruction (exception-safe) - * COPYRIGHT: Copyright 2009 Colin Finck (colin@reactos.org) + * COPYRIGHT: Copyright 2009-2019 Colin Finck (colin@reactos.org) */ #include "precomp.h" @@ -27,10 +27,11 @@ CProcess::CProcess(const wstring& CommandLine, LPSTARTUPINFOW StartupInfo) } /** - * Destructs a CProcess object and closes all handles belonging to the process. + * Destructs a CProcess object, terminates the process if running, and closes all handles belonging to the process. */ CProcess::~CProcess() { + TerminateProcess(m_ProcessInfo.hProcess, 255); CloseHandle(m_ProcessInfo.hThread); CloseHandle(m_ProcessInfo.hProcess); } diff --git a/modules/rostests/rosautotest/CWineTest.cpp b/modules/rostests/rosautotest/CWineTest.cpp index baf68a6b8a3..6e163979e0b 100644 --- a/modules/rostests/rosautotest/CWineTest.cpp +++ b/modules/rostests/rosautotest/CWineTest.cpp @@ -2,12 +2,13 @@ * PROJECT: ReactOS Automatic Testing Utility * LICENSE: GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+) * PURPOSE: Class implementing functions for handling Wine tests - * COPYRIGHT: Copyright 2009-2015 Colin Finck (colin@reactos.org) + * COPYRIGHT: Copyright 2009-2019 Colin Finck (colin@reactos.org) */ #include "precomp.h" static const DWORD ListTimeout = 10000; +static const DWORD ProcessActivityTimeout = 120000; /** * Constructs a CWineTest object. @@ -140,7 +141,7 @@ CWineTest::DoListCommand() /* Read the data */ m_ListBuffer = new char[BytesAvailable]; - if(!Pipe.Read(m_ListBuffer, BytesAvailable, &Temp)) + if(Pipe.Read(m_ListBuffer, BytesAvailable, &Temp, INFINITE) != ERROR_SUCCESS) TESTEXCEPTION("CPipe::Read failed\n"); return BytesAvailable; @@ -291,17 +292,34 @@ CWineTest::RunTest(CTestInfo* TestInfo) CPipedProcess Process(TestInfo->CommandLine, Pipe); /* Receive all the data from the pipe */ - while(Pipe.Read(Buffer, sizeof(Buffer) - 1, &BytesAvailable) && BytesAvailable) + for (;;) { - /* Output text through StringOut, even while the test is still running */ - Buffer[BytesAvailable] = 0; - tailString = StringOut(tailString.append(string(Buffer)), false); + DWORD dwReadResult = Pipe.Read(Buffer, sizeof(Buffer) - 1, &BytesAvailable, ProcessActivityTimeout); + if (dwReadResult == ERROR_SUCCESS) + { + /* 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; + } + else if (dwReadResult == ERROR_BROKEN_PIPE) + { + // The process finished and has been terminated. + break; + } + else if (dwReadResult == WAIT_TIMEOUT) + { + // The process activity timeout above has elapsed without any new data. + TESTEXCEPTION("Timeout while waiting for the test process\n"); + } + else + { + // An unexpected error. + TESTEXCEPTION("CPipe::Read failed for the test run\n"); + } } - if(GetLastError() != ERROR_BROKEN_PIPE) - TESTEXCEPTION("CPipe::Read failed for the test run\n"); } catch(CTestException& e) {