-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix reporting of an async IO timeout error on Windows (SerialPort
)
#81744
Conversation
- When an async `SerialPort` IO operation times out, it reports the timeout in the IO completion with an `NTSTATUS` value of `WAIT_TIMEOUT` (258) - In the thread pool when using `GetQueuedCompletionStatusEx`, the `NTSTATUS` value was being checked against `STATUS_SUCCESS` to determine success, so the `WAIT_TIMEOUT` was reported as an error. This leads to a different exception being thrown, compared to before when `GetQueuedCompletionStatus` was used. - Fixed to use similar logic to the SDK's `NT_SUCCESS` macro, which treats the `WAIT_TIMEOUT` value as a success, which is similar to what `GetQueuedCompletionStatus` does - There are already tests that verify this behavior in `System.IO.Ports` tests, though [they are currently disabled](https://github.com/dotnet/runtime/blob/b39d6a6eb44860746e91e5ce4f585beff33d1f63/src/libraries/System.IO.Ports/tests/Support/TCSupport.cs#L108-L118) due to instabilities. I have verified locally that the relevant failures are fixed and that there are no new failures in those tests. Fixes dotnet#80079
Tagging subscribers to this area: @mangod9 Issue Details
Fixes #80079
|
…ort`) - Port of dotnet#81744 - When an async `SerialPort` IO operation times out, it reports the timeout in the IO completion with an `NTSTATUS` value of `WAIT_TIMEOUT` (258) - In the thread pool when using `GetQueuedCompletionStatusEx`, the `NTSTATUS` value was being checked against `STATUS_SUCCESS` to determine success, so the `WAIT_TIMEOUT` was reported as an error. This leads to a different exception being thrown, compared to before when `GetQueuedCompletionStatus` was used. - Fixed to use similar logic to the SDK's `NT_SUCCESS` macro, which treats the `WAIT_TIMEOUT` value as a success, which is similar to what `GetQueuedCompletionStatus` does - There are already tests that verify this behavior in `System.IO.Ports` tests, though [they are currently disabled](https://github.com/dotnet/runtime/blob/b39d6a6eb44860746e91e5ce4f585beff33d1f63/src/libraries/System.IO.Ports/tests/Support/TCSupport.cs#L108-L118) due to instabilities. I have verified locally that the relevant failures are fixed and that there are no new failures in those tests. Relevant to dotnet#80079
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, FWIW I've checked other usages of STATUS_SUCCESS
in System.IO* and they LGTM.
@kouvel big thanks for the fix!
…ort`) (#81745) - Port of #81744 - When an async `SerialPort` IO operation times out, it reports the timeout in the IO completion with an `NTSTATUS` value of `WAIT_TIMEOUT` (258) - In the thread pool when using `GetQueuedCompletionStatusEx`, the `NTSTATUS` value was being checked against `STATUS_SUCCESS` to determine success, so the `WAIT_TIMEOUT` was reported as an error. This leads to a different exception being thrown, compared to before when `GetQueuedCompletionStatus` was used. - Fixed to use similar logic to the SDK's `NT_SUCCESS` macro, which treats the `WAIT_TIMEOUT` value as a success, which is similar to what `GetQueuedCompletionStatus` does - There are already tests that verify this behavior in `System.IO.Ports` tests, though [they are currently disabled](https://github.com/dotnet/runtime/blob/b39d6a6eb44860746e91e5ce4f585beff33d1f63/src/libraries/System.IO.Ports/tests/Support/TCSupport.cs#L108-L118) due to instabilities. I have verified locally that the relevant failures are fixed and that there are no new failures in those tests. Relevant to #80079
The failure looks like #81123 and is unrelated. |
SerialPort
IO operation times out, it reports the timeout in the IO completion with anNTSTATUS
value ofWAIT_TIMEOUT
(258)GetQueuedCompletionStatusEx
, theNTSTATUS
value was being checked againstSTATUS_SUCCESS
to determine success, so theWAIT_TIMEOUT
was reported as an error. This leads to a different exception being thrown, compared to before whenGetQueuedCompletionStatus
was used.NT_SUCCESS
macro, which treats theWAIT_TIMEOUT
value as a success, which is similar to whatGetQueuedCompletionStatus
doesSystem.IO.Ports
tests, though they are currently disabled due to instabilities. I have verified locally that the relevant failures are fixed and that there are no new failures in those tests.Fixes #80079