From 84882e0b52b7931b96dd004de6392368f2438f8e Mon Sep 17 00:00:00 2001 From: Bryan Roe Date: Fri, 23 Aug 2019 17:07:12 -0700 Subject: [PATCH] Fixed the following bugs on Windows: 1. Normally when a process exits when a stream is paused, I delayed the exit propagation until resume was called, so data was not lost. However, if no data was read, resume may not get called, causing the descriptor to continually trigger the event loop. For example, if the user explicity killed the process, resume won't get triggered after the fact. 2. If the above descriptor continually triggers the event loop, other descriptors will not get serviced, which can cause a stall 3. The above two issues caused KVM issues on windows, where it would not clean up properly and leak handles because the exit wasn't gettign called. But at the same time there's a race condition depending on where in the event list the descriptor is, could cause future KVM sessions to stall. --- microstack/ILibProcessPipe.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/microstack/ILibProcessPipe.c b/microstack/ILibProcessPipe.c index 2994a5a..6e3ea53 100644 --- a/microstack/ILibProcessPipe.c +++ b/microstack/ILibProcessPipe.c @@ -1034,7 +1034,7 @@ void ILibProcessPipe_Process_ReadHandler(void* user) #endif ILibProcessPipe_PipeObject *pipeObject = (ILibProcessPipe_PipeObject*)user; int consumed; - int err; + int err=0; #ifdef WIN32 BOOL result; @@ -1048,7 +1048,11 @@ void ILibProcessPipe_Process_ReadHandler(void* user) { #ifdef WIN32 result = GetOverlappedResult(pipeObject->mPipe_ReadEnd, pipeObject->mOverlapped, &bytesRead, FALSE); - if (result == FALSE) { break; } + if (result == FALSE) + { + err = GetLastError(); + break; + } #else bytesRead = (int)read(pipeObject->mPipe_ReadEnd, pipeObject->buffer + pipeObject->readOffset + pipeObject->totalRead, pipeObject->bufferSize - pipeObject->totalRead); if(bytesRead <= 0) @@ -1122,7 +1126,7 @@ void ILibProcessPipe_Process_ReadHandler(void* user) } #ifdef WIN32 while ((result = ReadFile(pipeObject->mPipe_ReadEnd, pipeObject->buffer + pipeObject->readOffset + pipeObject->totalRead, pipeObject->bufferSize - pipeObject->totalRead, NULL, pipeObject->mOverlapped)) == TRUE); // Note: This is actually the end of a do-while loop - if (pipeObject->PAUSED == 0 && (err = GetLastError()) != ERROR_IO_PENDING) + if((err = GetLastError()) != ERROR_IO_PENDING && (pipeObject->PAUSED == 0 || pipeObject->totalRead == 0)) #else while(pipeObject->PAUSED == 0); // Note: This is actually the end of a do-while loop err = 0; @@ -1573,7 +1577,7 @@ BOOL ILibProcessPipe_Process_OnExit(HANDLE event, ILibWaitHandle_ErrorStatus err UNREFERENCED_PARAMETER(event); ILibProcessPipe_WaitHandle_Remove(j->parent, j->hProcess); - if (j->stdOut->PAUSED != 0 || j->stdErr->PAUSED != 0) + if ((j->stdOut->PAUSED != 0 && j->stdOut->totalRead > 0) || (j->stdErr->PAUSED != 0 && j->stdErr->totalRead > 0)) { j->hProcess_needAdd = 1; }