https://git.reactos.org/?p=reactos.git;a=commitdiff;h=7dd4d2256b97b1f8a3a392060592a3d0f715fbd7
commit 7dd4d2256b97b1f8a3a392060592a3d0f715fbd7
Author: Colin Finck <co...@reactos.org>
AuthorDate: Tue Apr 23 09:17:05 2019 +0200
Commit: Timo Kreuzer <timo.kreu...@reactos.org>
CommitDate: Fri Apr 26 08:47:15 2019 +0200
[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.
---
modules/rostests/rosautotest/CPipe.cpp | 101 +++++++++++++++++++++++++++--
modules/rostests/rosautotest/CPipe.h | 6 +-
modules/rostests/rosautotest/CProcess.cpp | 5 +-
modules/rostests/rosautotest/CWineTest.cpp | 38 ++++++++---
4 files changed, 131 insertions(+), 19 deletions(-)
diff --git a/modules/rostests/rosautotest/CPipe.cpp
b/modules/rostests/rosautotest/CPipe.cpp
index 8fd3cf03e1..5af7163971 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.fa...@reactos.org)
+ * Copyright 2019 Colin Finck (co...@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 6efe670373..84bb01897f 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.fa...@reactos.org)
+ * Copyright 2019 Colin Finck (co...@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 65445cb379..e462456564 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 (co...@reactos.org)
+ * COPYRIGHT: Copyright 2009-2019 Colin Finck (co...@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 baf68a6b8a..6e163979e0 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 (co...@reactos.org)
+ * COPYRIGHT: Copyright 2009-2019 Colin Finck (co...@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)
{
