Fix a possible concurrency issue with CancelIo and winpty shutdown.

CancelIo requests that the I/O operation end, but the I/O operation may
still be going when the function returns.

https://blogs.msdn.microsoft.com/oldnewthing/20110202-00/?p=11613/
This commit is contained in:
Ryan Prichard 2015-12-28 21:49:41 -06:00
parent 0fb98c03fa
commit f5e8b4cd28
4 changed files with 17 additions and 0 deletions

View File

@ -121,6 +121,18 @@ int NamedPipe::IoWorker::service()
return progress; return progress;
} }
// This function is called after CancelIo has returned. We need to block until
// the I/O operations have completed, which should happen very quickly.
// https://blogs.msdn.microsoft.com/oldnewthing/20110202-00/?p=11613
void NamedPipe::IoWorker::waitForCanceledIo()
{
if (m_pending) {
DWORD actual = 0;
GetOverlappedResult(m_namedPipe->m_handle, &m_over, &actual, TRUE);
m_pending = false;
}
}
HANDLE NamedPipe::IoWorker::getWaitEvent() HANDLE NamedPipe::IoWorker::getWaitEvent()
{ {
return m_pending ? m_event : NULL; return m_pending ? m_event : NULL;
@ -247,6 +259,8 @@ void NamedPipe::closePipe()
if (m_handle == NULL) if (m_handle == NULL)
return; return;
CancelIo(m_handle); CancelIo(m_handle);
m_inputWorker->waitForCanceledIo();
m_outputWorker->waitForCanceledIo();
delete m_inputWorker; delete m_inputWorker;
delete m_outputWorker; delete m_outputWorker;
CloseHandle(m_handle); CloseHandle(m_handle);

View File

@ -43,6 +43,7 @@ private:
IoWorker(NamedPipe *namedPipe); IoWorker(NamedPipe *namedPipe);
virtual ~IoWorker(); virtual ~IoWorker();
int service(); int service();
void waitForCanceledIo();
HANDLE getWaitEvent(); HANDLE getWaitEvent();
protected: protected:
NamedPipe *m_namedPipe; NamedPipe *m_namedPipe;

View File

@ -109,6 +109,7 @@ void InputHandler::threadProc() {
trace("InputHandler: shutting down, canceling I/O"); trace("InputHandler: shutting down, canceling I/O");
assert(m_shouldShutdown); assert(m_shouldShutdown);
CancelIo(m_winpty); CancelIo(m_winpty);
GetOverlappedResult(m_winpty, &over, &written, TRUE);
break; break;
} }
assert(waitRet == WAIT_OBJECT_0); assert(waitRet == WAIT_OBJECT_0);

View File

@ -83,6 +83,7 @@ void OutputHandler::threadProc() {
trace("OutputHandler: shutting down, canceling I/O"); trace("OutputHandler: shutting down, canceling I/O");
assert(m_shouldShutdown); assert(m_shouldShutdown);
CancelIo(m_winpty); CancelIo(m_winpty);
GetOverlappedResult(m_winpty, &over, &numRead, TRUE);
break; break;
} }
assert(waitRet == WAIT_OBJECT_0); assert(waitRet == WAIT_OBJECT_0);