dotMorten/NmeaParser

NmeaDevice CloseAsync hangs on .NET 5

Closed this issue · 5 comments

Hi,

I had converted my project from .Net Framework 4.8 to .Net 5. Somehow the CloseAsync method of a NmeaDevice hangs the application.
The application can open one or more Forms with a NmeaDevice. When the Form closes, the device is closed. This is when the application hangs.

I figured out that the problem was/is the line: await m_ParserTask;
I don't know why this hangs the application, but it will never stop waiting. Might be due to .Net 5 since the application in .Net Framework did not have this issue.

The solution in the forked project is to use the following code.

if (m_ParserTask != null) { try { m_ParserTask.Wait(); } catch (OperationCanceledException e) { } }

On some occasions the Wait throws an OperationCanceledException. This one is catched without action.

Also, the CancellationTokenSource class implements the IDispose interface. It's best to Dispose the m_cts reference like: m_cts?.Dispose();

With this code the application works fine.
Maybe you can use this code(?)

Could you please provide a small sample that reproduces the issue? There's quite a lot of info lacking around what you're using, how you're using it. which version etc. I upgraded the sample app in this repro to .NET 5 and was not seeing the issue you report.

Also are you referring to this line?

if (m_stream != null)
await CloseStreamAsync(m_stream);

I can for sure say that using .Wait() is not a solid option as a fix, as it could cause a serious deadlock in your application. It might seem to work for you but there could be some very bad side effects and cause random application hangs.

Hi,

I tried but could not reproduce the issue with a new test project.

I also had trouble with some other async stuff from the Windows.Device namespace. The application also got stuck after multiple calls to async methods from this namesapace objects. After I solved those issues the issue with your NmeaDevice dissapeared.
The conclusion must be that I managed to have caused a deadlock preventing the await m_ParserTask; from ending.

I'm sorry for the confusion and thanks for your quick reaction. And ofcouse also thanks for the component!
Please disregard this issue.

Here's a simple unit test I wrote that simulates what's happening with the Nmea and SerialPort device classes.

I believe this is still an issue on the latest package 2.2.2 package from Nuget.

If you read this article, it might lead to believe that it's an issue with the serial port class that we wouldn't have any control over (other than working around it in the SerialPortDevice class somehow...)

Gotta go do other things, but am game to talk about this as well as how I might implement a PR for this.

Here's the code:
it's also important to note that COM1 did not have the GPS connected to it. I am playing around with implementing a kind of "COM port crawler" that's built on top of the NmeaParser lib layer. (I get that there are probably more efficient ways to do this.... but still I think this issue could arise for others, especially if the device on the other end is not writing data to the stream)

       /*xUnit test*/ [Fact]
        public async Task Connect_Read_Disconnect_Hangs()
        {
            //arrange
            var portName = "COM1";
            var baudRate = 4800;
            var serialPort = new System.IO.Ports.SerialPort(portName, baudRate);

            var cancellationTokenSource = new CancellationTokenSource();
            var cancellationToken = cancellationTokenSource.Token;

            //act

            //simulate NmeaParser.SerialPortDevice.OpenAsync()
            serialPort.Open();
            var stream = serialPort.BaseStream;

            var readTask = Task.Run(async () =>
            {
                byte[] buffer = new byte[1024];
                int readCount = 0;
                while (!cancellationTokenSource.IsCancellationRequested)
                {
                    _ = await stream.ReadAsync(buffer, 0, buffer.Length).ConfigureAwait(false);
                    if (cancellationToken.IsCancellationRequested)
                        break;

                }
                await Task.Yield();
            });

            await Task.Delay(1000 * 10);
            //simulate NmeaParser.SerialPortDevice.CloseAsync()
            if (cancellationTokenSource != null)
            {
                cancellationTokenSource.Cancel();
                cancellationTokenSource = null;
            }

            if (readTask != null && !readTask.IsCompleted)
            {
                await readTask;
            }

            if (stream != null)
            {
                if(serialPort.IsOpen)
                    serialPort.Close();
            }
            stream = null;

        }

p.s. this library is awesome!!

@SonicScholar since this issue has been closed for a long time, please log a new issue so this doesn't get lost

Yep, fair enough. Opened up #113