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
624 stars 197 forks source link

SerialPortStream.IsOpen throws NullreferenceException #90

Closed JesperNJessen closed 4 years ago

JesperNJessen commented 5 years ago

On occasion, the IsOpen property will throw a NullreferenceException. I have not been able to reliably reproduce the issue, but it happens 1-2 times per day during normal use.

FrankHileman commented 5 years ago

I found code in WinNativeSerial that could cause this exception. The following IsOpen code in the native property will cause problems across threads: return m_ComPortHandle != null && !m_ComPortHandle.IsClosed && !m_ComPortHandle.IsInvalid; The field can be null between the time it is checked for null and the time the IsClosed is accessed. A safer technique, for all similar properties that can be nulled: var handle = m_ComPortHandle; return handle != null && !handle.IsClosed && !handle.IsInvalid I suggest to review all nullable, mutable fields.

jcurl commented 4 years ago

It's been a while. I'm going to modify the code so that IsOpen doesn't throw a NullReferenceException. However, there is generally no expectation that after a Close() the API will not throw an exception if not synchronized.

So that means if one thread is polling with IsOpen and then if the result is true, it will do something, this will not work, unless there is a critical section implemented preventing Close() from being called from user code while the other API is in use.

jcurl commented 4 years ago

Released in v2.2.1 on NuGet