Skip to content
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

Breaking change with timeout reading serial port in .NET 7 (Possible Overlapped I/O change) #80079

Closed
dementedmonkey opened this issue Dec 31, 2022 · 10 comments · Fixed by #81744

Comments

@dementedmonkey
Copy link

Description

In .NET 6 and earlier, when System.IO.Ports.SerialPort.Read times out, it will throw a TimeoutException. In .NET 7, it will throw an IOException with an HResult of 0x800705B4 (1460: ERROR_TIMEOUT)

Reproduction Steps

Use the following code (substitute your own valid serial port values)

  • VS2022 or .NET 7 SDK on Windows.
  • Nuget Reference to System.IO.Ports version 7.0.0 (OK with eariler versions too)
  • Target net7.0 to see it fail, or net6.0 to see it work.
using System;
using System.IO;
using System.IO.Ports;

class Program
{
    static void Main()
    {
        var serialPort = new SerialPort("COM42", 115200, Parity.None, 8, StopBits.One)
        {
            DtrEnable = false,
            WriteTimeout = 500,
            ReadTimeout = 500,
        };
        serialPort.Open();
        var data = new byte[100];
        try
        {
            var numRead = serialPort.Read(data, 0, data.Length);
            Console.WriteLine("Read {0}", numRead);
        }
        catch (TimeoutException)
        {
            Console.WriteLine(".NET <=6 Timed out");
        }
        catch (IOException e)
        {
            Console.WriteLine(".NET 7: IO {0}  HR:0x{1:X}", e.HResult & 0xFFFF, e.HResult);
        }
    }
}

Expected behavior

Output of .NET <=6 Timed out

Actual behavior

Output of .NET 7: IO 1460 HR:0x800705B4

Regression?

Yes. Worked for .NET 6 and 5.

You can easily test this but switching target framework. If you switch to .NET 6 you get the TimeoutException, if you switch to .NET 7 you get the IOException

Known Workarounds

Code catching the TimeoutException would have to catch the IOException check the HResult and handle it the same way.

Configuration

.net 7.0.101 Windows 11, Target X64 or x86 has the problem.
Debug or release doesn't seem to matter.

Ubuntu WSL2 with 7.0.101 does not have the problem.

Other information

This seems to be caused by changed in overlapped IO introduced in .net7

The difference can be observed here:


On .net6, the errorCode is 0, on .net7 it's 1460

The call stack invoking this function is very different on .net 6 and 7.

On .net7, AsyncFSCallback is getttiong called from PortableThreadPool and the error is set here:

errorCode = Interop.NtDll.RtlNtStatusToDosError((int)ntStatus);

On .net6, it's getting called from _IOCompletionCallback.PerformIOCompletionCallback The stack stops there, and I didn't dig any deeper to see where it's getting its parameters from.

@krwq
Copy link
Member

krwq commented Jan 13, 2023

See workaround for this issue in #78319 (note it was closed as dup since this one has more details)

@krwq krwq added this to the 8.0.0 milestone Jan 13, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 13, 2023
@krwq krwq added the bug label Jan 13, 2023
@krwq
Copy link
Member

krwq commented Jan 13, 2023

Given System.IO.Ports is a nuget package and there is a workaround I suggest we just do fix in vNext.

@AlquistArj
Copy link

I also have this issue with .NET 7 serial port code (it used to work in .NET 4.8).

@hugo-lyppens
Copy link

What is the ETA for the fix? Is it just going to be a new https://nuget.info/packages/System.IO.Ports/7.0.0 or a .NET 7 fix?

@krwq
Copy link
Member

krwq commented Jan 24, 2023

@hugo-lyppens I can't promise anything specific to a timeline.

I couldn't pinpoint any specific change which could have regressed this so I've done some sanity testing and here are my findings:

Target Framework System.IO.Ports version Exception
6.0 6.0 TimeoutException
6.0 7.0 TimeoutException
7.0 6.0 IOException
7.0 7.0 IOException

Given above this doesn't look like regression was in System.IO.Ports and was likely somewhere on System.IO layer.
I'm not sure if change in System.IO was intentional or not and if we should try to workaround it in System.IO.Ports.

@dotnet/area-system-io will need to take a look and from there we can decide on the next steps

@ghost
Copy link

ghost commented Jan 24, 2023

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

In .NET 6 and earlier, when System.IO.Ports.SerialPort.Read times out, it will throw a TimeoutException. In .NET 7, it will throw an IOException with an HResult of 0x800705B4 (1460: ERROR_TIMEOUT)

Reproduction Steps

Use the following code (substitute your own valid serial port values)

  • VS2022 or .NET 7 SDK on Windows.
  • Nuget Reference to System.IO.Ports version 7.0.0 (OK with eariler versions too)
  • Target net7.0 to see it fail, or net6.0 to see it work.
using System;
using System.IO;
using System.IO.Ports;

class Program
{
    static void Main()
    {
        var serialPort = new SerialPort("COM42", 115200, Parity.None, 8, StopBits.One)
        {
            DtrEnable = false,
            WriteTimeout = 500,
            ReadTimeout = 500,
        };
        serialPort.Open();
        var data = new byte[100];
        try
        {
            var numRead = serialPort.Read(data, 0, data.Length);
            Console.WriteLine("Read {0}", numRead);
        }
        catch (TimeoutException)
        {
            Console.WriteLine(".NET <=6 Timed out");
        }
        catch (IOException e)
        {
            Console.WriteLine(".NET 7: IO {0}  HR:0x{1:X}", e.HResult & 0xFFFF, e.HResult);
        }
    }
}

Expected behavior

Output of .NET <=6 Timed out

Actual behavior

Output of .NET 7: IO 1460 HR:0x800705B4

Regression?

Yes. Worked for .NET 6 and 5.

You can easily test this but switching target framework. If you switch to .NET 6 you get the TimeoutException, if you switch to .NET 7 you get the IOException

Known Workarounds

Code catching the TimeoutException would have to catch the IOException check the HResult and handle it the same way.

Configuration

.net 7.0.101 Windows 11, Target X64 or x86 has the problem.
Debug or release doesn't seem to matter.

Ubuntu WSL2 with 7.0.101 does not have the problem.

Other information

This seems to be caused by changed in overlapped IO introduced in .net7

The difference can be observed here:


On .net6, the errorCode is 0, on .net7 it's 1460

The call stack invoking this function is very different on .net 6 and 7.

On .net7, AsyncFSCallback is getttiong called from PortableThreadPool and the error is set here:

errorCode = Interop.NtDll.RtlNtStatusToDosError((int)ntStatus);

On .net6, it's getting called from _IOCompletionCallback.PerformIOCompletionCallback The stack stops there, and I didn't dig any deeper to see where it's getting its parameters from.

Author: dementedmonkey
Assignees: -
Labels:

bug, area-System.IO, regression-from-last-release

Milestone: 8.0.0

@jkotas
Copy link
Member

jkotas commented Jan 24, 2023

This is likely caused by a switch to portable thread pool. cc @kouvel

@ghost
Copy link

ghost commented Jan 24, 2023

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

In .NET 6 and earlier, when System.IO.Ports.SerialPort.Read times out, it will throw a TimeoutException. In .NET 7, it will throw an IOException with an HResult of 0x800705B4 (1460: ERROR_TIMEOUT)

Reproduction Steps

Use the following code (substitute your own valid serial port values)

  • VS2022 or .NET 7 SDK on Windows.
  • Nuget Reference to System.IO.Ports version 7.0.0 (OK with eariler versions too)
  • Target net7.0 to see it fail, or net6.0 to see it work.
using System;
using System.IO;
using System.IO.Ports;

class Program
{
    static void Main()
    {
        var serialPort = new SerialPort("COM42", 115200, Parity.None, 8, StopBits.One)
        {
            DtrEnable = false,
            WriteTimeout = 500,
            ReadTimeout = 500,
        };
        serialPort.Open();
        var data = new byte[100];
        try
        {
            var numRead = serialPort.Read(data, 0, data.Length);
            Console.WriteLine("Read {0}", numRead);
        }
        catch (TimeoutException)
        {
            Console.WriteLine(".NET <=6 Timed out");
        }
        catch (IOException e)
        {
            Console.WriteLine(".NET 7: IO {0}  HR:0x{1:X}", e.HResult & 0xFFFF, e.HResult);
        }
    }
}

Expected behavior

Output of .NET <=6 Timed out

Actual behavior

Output of .NET 7: IO 1460 HR:0x800705B4

Regression?

Yes. Worked for .NET 6 and 5.

You can easily test this but switching target framework. If you switch to .NET 6 you get the TimeoutException, if you switch to .NET 7 you get the IOException

Known Workarounds

Code catching the TimeoutException would have to catch the IOException check the HResult and handle it the same way.

Configuration

.net 7.0.101 Windows 11, Target X64 or x86 has the problem.
Debug or release doesn't seem to matter.

Ubuntu WSL2 with 7.0.101 does not have the problem.

Other information

This seems to be caused by changed in overlapped IO introduced in .net7

The difference can be observed here:


On .net6, the errorCode is 0, on .net7 it's 1460

The call stack invoking this function is very different on .net 6 and 7.

On .net7, AsyncFSCallback is getttiong called from PortableThreadPool and the error is set here:

errorCode = Interop.NtDll.RtlNtStatusToDosError((int)ntStatus);

On .net6, it's getting called from _IOCompletionCallback.PerformIOCompletionCallback The stack stops there, and I didn't dig any deeper to see where it's getting its parameters from.

Author: dementedmonkey
Assignees: -
Labels:

bug, area-System.IO, area-System.Threading, regression-from-last-release

Milestone: 8.0.0

kouvel added a commit to kouvel/runtime that referenced this issue Feb 7, 2023
- 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
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 7, 2023
kouvel added a commit to kouvel/runtime that referenced this issue Feb 7, 2023
…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
@kouvel
Copy link
Member

kouvel commented Feb 7, 2023

The NTSTATUS value from the IO completion is 258 (WAIT_TIMEOUT). In .NET 6 GetQueuedCompletionStatus treats that value as a success. In .NET 7 the value is incorrectly treated and reported as an error by the portable thread pool, leading to a different exception.

A workaround is to set the environment variable DOTNET_ThreadPool_UsePortableThreadPoolForIO=0 before starting the app.

I have put up PRs for fixes in #81744 (main) and #81745 (7.0).

@kouvel
Copy link
Member

kouvel commented Feb 7, 2023

What is the ETA for the fix? Is it just going to be a new https://nuget.info/packages/System.IO.Ports/7.0.0 or a .NET 7 fix?

It's a .NET 7 fix. If the 7.0 PR is approved it'll be marked with a milestone indicating the runtime version that would include the fix.

carlossanlop pushed a commit that referenced this issue Feb 9, 2023
…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
kouvel added a commit that referenced this issue Feb 15, 2023
…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.

Fixes #80079
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 15, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants