Skip to content

Commit

Permalink
Modified dotnet#41008 without GetOverlappedResultEx (dotnet#41236)
Browse files Browse the repository at this point in the history 
* Revert "Revert "Improve Windows error handling in Diagnostics IPC (dotnet#41008)" (dotnet#41220)"

This reverts commit 15839c0.

* don't use win8+ API
  • Loading branch information
John Salem committed Aug 25, 2020
1 parent 869aaac commit 8242a72
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 35 deletions.
73 changes: 40 additions & 33 deletions src/coreclr/src/debug/debug-pal/win/diagnosticsipc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,12 +393,12 @@ int32_t IpcStream::DiagnosticsIpc::Poll(IpcPollHandle *rgIpcPollHandles, uint32_
if (!fSuccess)
{
DWORD error = ::GetLastError();
if (error == ERROR_PIPE_NOT_CONNECTED)
if (error == ERROR_PIPE_NOT_CONNECTED || error == ERROR_BROKEN_PIPE)
rgIpcPollHandles[index].revents = (uint8_t)IpcStream::DiagnosticsIpc::PollEvents::HANGUP;
else
{
if (callback != nullptr)
callback("Client connection error", -1);
callback("Client connection error", error);
rgIpcPollHandles[index].revents = (uint8_t)IpcStream::DiagnosticsIpc::PollEvents::ERR;
delete[] pHandles;
return -1;
Expand Down Expand Up @@ -434,39 +434,43 @@ bool IpcStream::Read(void *lpBuffer, const uint32_t nBytesToRead, uint32_t &nByt

if (!fSuccess)
{
// if we're waiting infinitely, only make one syscall
if (timeoutMs == InfiniteTimeout)
{
fSuccess = GetOverlappedResult(_hPipe,
overlap,
&nNumberOfBytesRead,
true) != 0;
fSuccess = GetOverlappedResult(_hPipe, // pipe
overlap, // overlapped
&nNumberOfBytesRead, // out actual number of bytes read
true) != 0; // block until async IO completes
}
else
{
DWORD dwError = GetLastError();
if (dwError == ERROR_IO_PENDING)
{
// Wait on overlapped IO event (triggers when async IO is complete regardless of success)
// or timeout
DWORD dwWait = WaitForSingleObject(_oOverlap.hEvent, (DWORD)timeoutMs);
if (dwWait == WAIT_OBJECT_0)
{
// get the result
fSuccess = GetOverlappedResult(_hPipe,
overlap,
&nNumberOfBytesRead,
true) != 0;
// async IO compelted, get the result
fSuccess = GetOverlappedResult(_hPipe, // pipe
overlap, // overlapped
&nNumberOfBytesRead, // out actual number of bytes read
true) != 0; // block until async IO completes
}
else
{
// cancel IO and ensure the cancel happened
if (CancelIo(_hPipe))
// We either timed out or something else went wrong.
// For any error, attempt to cancel IO and ensure the cancel happened
if (CancelIoEx(_hPipe, overlap) != 0)
{
// check if the async write beat the cancellation
fSuccess = GetOverlappedResult(_hPipe, overlap, &nNumberOfBytesRead, true) != 0;
// Failure here isn't recoverable, so return as such
}
}
}
}
// TODO: Add error handling.
}

nBytesRead = static_cast<uint32_t>(nNumberOfBytesRead);
Expand All @@ -488,40 +492,43 @@ bool IpcStream::Write(const void *lpBuffer, const uint32_t nBytesToWrite, uint32

if (!fSuccess)
{
DWORD dwError = GetLastError();
if (dwError == ERROR_IO_PENDING)
// if we're waiting infinitely, only make one syscall
if (timeoutMs == InfiniteTimeout)
{
if (timeoutMs == InfiniteTimeout)
{
// if we're waiting infinitely, don't bother with extra kernel call
fSuccess = GetOverlappedResult(_hPipe,
overlap,
&nNumberOfBytesWritten,
true) != 0;
}
else
fSuccess = GetOverlappedResult(_hPipe, // pipe
overlap, // overlapped
&nNumberOfBytesWritten, // out actual number of bytes written
true) != 0; // block until async IO completes
}
else
{
DWORD dwError = GetLastError();
if (dwError == ERROR_IO_PENDING)
{
// Wait on overlapped IO event (triggers when async IO is complete regardless of success)
// or timeout
DWORD dwWait = WaitForSingleObject(_oOverlap.hEvent, (DWORD)timeoutMs);
if (dwWait == WAIT_OBJECT_0)
{
// get the result
fSuccess = GetOverlappedResult(_hPipe,
overlap,
&nNumberOfBytesWritten,
true) != 0;
// async IO compelted, get the result
fSuccess = GetOverlappedResult(_hPipe, // pipe
overlap, // overlapped
&nNumberOfBytesWritten, // out actual number of bytes written
true) != 0; // block until async IO completes
}
else
{
// cancel IO and ensure the cancel happened
if (CancelIo(_hPipe))
// We either timed out or something else went wrong.
// For any error, attempt to cancel IO and ensure the cancel happened
if (CancelIoEx(_hPipe, overlap) != 0)
{
// check if the async write beat the cancellation
fSuccess = GetOverlappedResult(_hPipe, overlap, &nNumberOfBytesWritten, true) != 0;
// Failure here isn't recoverable, so return as such
}
}
}
}
// TODO: Add error handling.
}

nBytesWritten = static_cast<uint32_t>(nNumberOfBytesWritten);
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/src/vm/ipcstreamfactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ IpcStream *IpcStreamFactory::GetNextAvailableStream(ErrorCallback callback)
{
case IpcStream::DiagnosticsIpc::PollEvents::HANGUP:
((DiagnosticPort*)(rgIpcPollHandles[i].pUserData))->Reset(callback);
STRESS_LOG2(LF_DIAGNOSTICS_PORT, LL_INFO10, "IpcStreamFactory::GetNextAvailableStream - HUP :: Poll attempt: %d, connection %d hung up.\n", nPollAttempts, i);
STRESS_LOG2(LF_DIAGNOSTICS_PORT, LL_INFO10, "IpcStreamFactory::GetNextAvailableStream - HUP :: Poll attempt: %d, connection %d hung up. Connect is reset.\n", nPollAttempts, i);
pollTimeoutMs = s_pollTimeoutMinMs;
break;
case IpcStream::DiagnosticsIpc::PollEvents::SIGNALED:
Expand All @@ -330,7 +330,8 @@ IpcStream *IpcStreamFactory::GetNextAvailableStream(ErrorCallback callback)
STRESS_LOG2(LF_DIAGNOSTICS_PORT, LL_INFO10, "IpcStreamFactory::GetNextAvailableStream - SIG :: Poll attempt: %d, connection %d signalled.\n", nPollAttempts, i);
break;
case IpcStream::DiagnosticsIpc::PollEvents::ERR:
STRESS_LOG2(LF_DIAGNOSTICS_PORT, LL_INFO10, "IpcStreamFactory::GetNextAvailableStream - ERR :: Poll attempt: %d, connection %d errored.\n", nPollAttempts, i);
((DiagnosticPort*)(rgIpcPollHandles[i].pUserData))->Reset(callback);
STRESS_LOG2(LF_DIAGNOSTICS_PORT, LL_INFO10, "IpcStreamFactory::GetNextAvailableStream - ERR :: Poll attempt: %d, connection %d errored. Connection is reset.\n", nPollAttempts, i);
fSawError = true;
break;
case IpcStream::DiagnosticsIpc::PollEvents::NONE:
Expand Down

0 comments on commit 8242a72

Please sign in to comment.