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

ReadAsync blocks for .NET Core #116

Closed meinsiedler closed 3 years ago

meinsiedler commented 3 years ago

Hi,

this is a follow up issue to #114 and to the PR #115.

First of all, thanks for integrating the PR #115 and providing a new NuGet version.

Today, we tried to upgrade the SerialPortsStream library to the new version 2.3.0 in our project and found out that we still have issues with .NET Core 3.1 and using the ReadAsync and WriteAsync methods.

We could reproduce the issue with a pretty straight-forward unit test: We set up writing to the stream on COM1 and reading from COM2 (COM1 and COM2 are connected). For the .NET Core 3.1, the ReadAsync blocks and never reads the bytes from the stream whereas for .NET 4.7.2 everything works as expected.

public async Task WriteAsync()
{
    using (var serialPortStreamWrite = new SerialPortStream("COM1", 9600, 8, Parity.None, StopBits.One))
    using (var serialPortStreamRead = new SerialPortStream("COM2", 9600, 8, Parity.None, StopBits.One))
    {
        serialPortStreamWrite.Open();
        serialPortStreamRead.Open();

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

        var bytes = new byte[] { 0x01, 0x02, 0x03 };
        await serialPortStreamWrite.WriteAsync(bytes, 0, bytes.Length);
        await serialPortStreamWrite.FlushAsync();

        // ReadAsync blocks here even if something gets written and flushed to the underlying COM device.
        // Fails for netcoreapp3.1, works with net472
        await readTask;

        Assert.That(buffer[0], Is.EqualTo(0x01));
        Assert.That(buffer[1], Is.EqualTo(0x02));
        Assert.That(buffer[2], Is.EqualTo(0x03));
    }
}

Since there are no unit tests specific to NETSTANDARD in this repo, I added the unit test in my own test repo: https://github.com/meinsiedler/SerialPortStreamTest

Do you have any idea or hint, what the problem could be and how to fix it?

Best regards

jcurl commented 3 years ago

Is your test using physical cables, or a driver like com0com for a virtual null on a single machine?

meinsiedler commented 3 years ago

We use a virtual driver on a single machine for the tests. (Free Virtual Serial Ports, https://freevirtualserialports.com/)

jcurl commented 3 years ago

There is one comment about the unit test in that it could potentially fail (but not block). The Read(), ReadAsync, BeginRead/EndRead methods only guarantee at least one byte read. That means you should put the read in a loop until you get the 3 bytes you're looking for.

It probably works in your test case using the virtual serial driver (it's more memory/packet based I'd suspect instead of being byte based, something that would be more obvious on real hardware).

But as I said, that shouldn't cause it to block - as that tells me no data is being received.

Can you set up your app.config to trace? See https://github.com/jcurl/SerialPortStream/wiki/Tracing

I apologize, that I have done almost nothing regarding .NET Core (and only worked with the desktop) and to make it a first class citizen would need some time. Getting traces with high verbosity would be very helpful. I can track down where the problem is.

meinsiedler commented 3 years ago

Hi,

I tried to enable the tracing for the .NET Core sample console app in my test repo but it seems like the tracing isn't supported in .NET Core (see this github issue). So, I can only provide the trace log only for .NET 4.7.2 which won't help much, since it works there.

I also tried to replace the ReadAsync with a Read like this:

var readTask = Task.Run(() => serialPortStreamRead.Read(buffer, 0, buffer.Length));

With this call, the code also works with .NET Core 3.1 too. So it seems like it has something to do with the async / Begin|EndRead|Write version.

Sorry that I can't provide any more logs. Please reach out if I can help otherwise. Also, no need to apologize, I'm happy that I get some help. 😊

jcurl commented 3 years ago

I've debugged it to the point that this is .NET Core specific. The code runs the Task.Run(). I can confirm from your test case that the three bytes are actually read here.

        private IAsyncResult InternalBeginRead(byte[] buffer, int offset, int count, AsyncCallback callback, object state)
        {
            if (m_Buffer == null || count == 0 || m_Buffer.Stream.WaitForRead(0)) {
                ...
            } else {
                ReadDelegate read = InternalBlockingRead;
#if NETSTANDARD15
                // No data in buffer, so we create a thread in the background
                return Task.Run(() => read(buffer, offset, count));
#else
                ...
#endif
            }
        }

We see that the InternalEndRead is not being called, and that's obvious because Task.Run(...) doesn't execute the callback. This is why it fails. I've confirmed this is the problem (i.e. now the test case passes). I need to clean up the code and fix it.

jcurl commented 3 years ago

Please test the attached NuGet package. When you verify it works, I'll submit it for v2.3.1.

rjcp.dll.serialportstream-2.3.1-beta.20210417.1.zip

meinsiedler commented 3 years ago

Hi @jcurl , I tested your attached NuGet package and I can confirm that it now works as expected. Many thanks for your efforts!

mjhadden commented 3 years ago

Hello @jcurl, I also had an issue with no data seen at the ReadAsync. I have tested your attached beta package and also found that it now works as expected. Thank you for the quick turn around.

jcurl commented 3 years ago

OK, I'll update the versions to 2.3.1 and prepare a new release and close this ticket when it's in NuGet. Thanks all for your testing.