jcurl / RJCP.DLL.SerialPortStream

SerialPortStream is an independent implementation of System.IO.Ports.SerialPort and SerialStream for better reliability and maintainability. Default branch is 2.x and now has support for Mono with help of a C library.
Microsoft Public License
631 stars 197 forks source link

Support for .NET Core 3.1? #114

Closed meinsiedler closed 3 years ago

meinsiedler commented 3 years ago

Hi,

we are having issues after upgrading from .NET 4.7 to .NET Core 3.1. With .NET Core 3.1 it seems like nothing gets written to the COM port. We don't receive any errors or exceptions but no data shows up.

The issue is easily reproducible with the following sample code:

static void Main(string[] args)
{
    Console.WriteLine("Start");

    using (var serialPortStream = new SerialPortStream("COM1", 9600, 8, Parity.None, StopBits.One))
    {
        serialPortStream.Open();

        Console.WriteLine("Serial Port Opened");

        Console.WriteLine("Wait for write. Enter any key.");
        Console.ReadKey();

        var bytes = new byte[] { 0x01, 0x02, 0x03 };

        serialPortStream.WriteAsync(bytes, 0, bytes.Length);
        serialPortStream.FlushAsync();

        Console.WriteLine("Finished.");
    }
}

When using <TargetFramework>net472</TargetFramework> in the .csproj file, everything works as expected and the bytes get written to the COM port. When switching to <TargetFramework>netcoreapp3.1</TargetFramework> no data gets written to the COM port.

We are using a serial port monitoring tool to check if any data gets written. In the .NET Core 3.1 case, the data doesn't show up in our monitoring tool.

We use Windows 10 and the latest SerialPortStream version 2.2.2.

Unfortunately, we can't provide any further details, since we don't receive any errors and the only change is the TargetFramework. If there's anything more we can provide, please let us know.

jcurl commented 3 years ago

Hi, do you have a test program that also includes the solution file? Either for VSCode or Visual Studio?

I suspect the problem is seen because SerialPortStream doesn't have native implementations for ReadAsync or WriteAsync, so the base Stream class needs to do this. If you look at the code, Read/Write is for all frameworks, BeginRead/BeginWrite is only for .NET Framework (and the framework has wrappers for ReadAsync/WriteAsync). .NET Core doesn't provide BeginRead/BeginWrite and there's no ReadAsync/WriteAsync which means that .NET Core isn't compatible with the current implementation.

Best would be to put a breakpoint in the SerialPortStream.Write method and see if it's being called.

meinsiedler commented 3 years ago

Hi,

thanks for your response. While trying to reproduce the issue again, I noticed that I forgot to await the two calls in the sample code above.

await serialPortStream.WriteAsync(bytes, 0, bytes.Length);
await serialPortStream.FlushAsync();

When awaiting the two calls, everything works as expected with netcoreapp3.1 too.

I will close the issue for now, because it seems that the issue lies somewhere else in our code base and isn't reproducible with a simple sample program.

Sorry for the inconvenience and thanks again.

jcurl commented 3 years ago

No problem, glad that in principle it should work.

meinsiedler commented 3 years ago

Hi,

we further investigated our issue and now we could really reproduce it with a simple sample program.

We have a complex program where we set up the reading and writing from the COM port in different threads so that we can provide bi-directional communication. With .NET 4.7 it was possible to await the ReadAsync in one thread and at the same time call the WriteAsync in another thread. This doesn't work any longer with .NET Core 3.1. The await WriteAsync call blocks and never finishes. It seems that the await ReadAsync call from the other thread waits for something to read and blocks the writing on the other thread.

static async Task Main(string[] args)
{
    Console.WriteLine("Start");

    using (var serialPortStream = new SerialPortStream("COM1", 9600, 8, Parity.None, StopBits.One))
    {
        serialPortStream.Open();

        Console.WriteLine("Serial Port Opened");

        var buffer = new byte[1024];
        var readTask = Task.Run(async () => await serialPortStream.ReadAsync(buffer, 0, buffer.Length));

        Console.WriteLine("Wait for write. Enter any key.");
        Console.ReadKey();

        await Task.Run(async () =>
        {
            var bytes = new byte[] { 0x01, 0x02, 0x03 };

            Console.WriteLine("Write started");
            await serialPortStream.WriteAsync(bytes, 0, bytes.Length);
            await serialPortStream.FlushAsync();
            Console.WriteLine("Write finished");
        });

        Console.WriteLine("Awaiting read...");

        await readTask;

        Console.WriteLine("Finished.");
    }
}

I've also attached the solution. When changing the <TargetFramework> in the .csproj file to net472 everything works as expected and we can write to the stream while waiting for reading from the stream.

SerialPortStreamTest.zip

jcurl commented 3 years ago

Hi, this is really starting to sound like something I wrote about 8 years ago. See https://www.codeproject.com/Tips/575618/Avoiding-Deadlocks-with-System-IO-Stream-BeginRead

I know the framework doesn't like read/write on the same thread. This is why I implemented the Begin|EndRead/Begin|EndWrite on .NET Desktop. But .NET Core doesn't have this and I suspect the framework is deadlocking itself.

So it looks like the only solution for .NET Core is to provide an implementation of the AsyncRead/Write. You could confirm this by providing a backtrace at the time of the deadlock. Secondly, if you have a small test program (I'm sorry - I'm very busy with my day job, and when I get time, I could use a test case that is confirmed as failing to ensure a solution).

meinsiedler commented 3 years ago

Hi, thank you for your effort! This is indeed an interesting article and sheds some light on the issue.

I could use a test case that is confirmed as failing to ensure a solution

In case you have overlooked it, I have attached a solution with a simple test in form of an executable console app in my previous comment. This should quickly show the difference between netcoreapp3.1 and net472. If it helps and is more practical, I'll to provide a unit test project with failing tests.

We are using your package professionally (not in a side/pet project) and are very interested in fixing the issue. We are more than happy to help wherever possible.

meinsiedler commented 3 years ago

Hi,

I created a repository with a unit test project which shows the issue: https://github.com/meinsiedler/SerialPortStreamTest

Please also take a look at the README where I describe my findings.

Please reach out to us if we can help.

jcurl commented 3 years ago

Hello, thanks for providing the repository. I've forked it,. Please understand that my professional situation requires more than usual engagement at the same time I'm spending on my non-work hours on training, so I don't know when I can get to this, probably not in the next few weeks.. If it is as I suspect, an implementation of ReadAsync/WriteAsync will need to be done, and as I use the .NET 4.0 solution, whatever is written will need to be compatible with that.

It shouldn't be too hard, the main I/O loop sets semaphores in the buffer object, it would be figuring out how to use that and synchronize with a Task object. As you've indicated that this is a professional package, you might want to consider forking and providing a solution yourself, or hiring someone to provide this solution depending on your needs. Looking at the code for BeginRead/BeginWrite would be a good place to start. The CancellationToken stuff might be harder.

I'll leave this ticket open, at least to highlight there is now a real-world usecase that requires Async (there's another ticket, but with no impact mentioned).

jcurl commented 3 years ago

Integrated with bc94184904364411ef0f5406c918b4873e0b70a2.