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

Closing/disposing a port doesn't throw an exception on blocking methods #4

Closed superware closed 8 years ago

superware commented 8 years ago

First let me say this library looks great, a very impressive achievement considering the history of System.IO.Ports.SerialPort.

Shouldn't blocking Read/Write/etc raise ObjectDisposedException (or at least IOException and then on consequent calls ObjectDisposedException) when the SerialPortStream is being closed/disposed?

jcurl commented 8 years ago

Close() and Dispose() are two distinct functionalities in this project, where Close() doesn't dispose and allows a Open() at a later time. Dispose() calls close and will raise an exception if you try to open later, as internal resources are then also disposed (unlike Close()).

So, on Close() I do not wish to have an exception raised. A read operation if blocking would return zero as there is no longer data in the buffer to read, and there's no point in waiting. On Write() we should get a TimeoutException.

Thus, on Dispose(), I close the serial port and any subsequent operations should raise an exception. If the MS implementation raises an exception in the scenario you describe on Dispose(), I could consider implementing this.

superware commented 8 years ago

So, on Close() I do not wish to have an exception raised.

What would you say about this:

Operation State Behavior
Read Closing/ed return zero
Read Disposing return zero
Read Disposed ObjectDisposedException
Write Closing/ed IOException
Write Disposing IOException
Write Disposed ObjectDisposedException

So a common use case might be:

while (true)
{
  try
  {
    count = stream.Read(...);
  }
  catch (IOException)
  {
    continue;
  }
  catch (ObjectDisposedException)
  {
    break;
  }
  if (count == 0)
    break;
  ...
}

Please see MSDN: "Read returns 0 only when there is no more data in the stream and no more is expected (such as a closed socket or end of file)". Better yet, see here exceptions table and remark notes.

Consider a custom class MyReader : BinaryReader, taking a SerialPortStream in it's constructor, and being used in a new thread, _baseStream.Read will block, but if the BinaryReader is disposed later on, there will be no way to return and Join that thread etc.

Thanks.

jcurl commented 8 years ago

I'm not sure about your comments regarding Read(), as it should already return 0 when there's a timeout or when there's no data left in the buffer. Before I make a change, I would like to set up some test cases using MS's and Mono's implementation. In principle, I'm raising exceptions (TimeoutException on Write() when closed, ObjectDisposed() when already disposed for read/write, no exception and always zero on Read() as per MSDN).

Just so I understand properly, your suggesting to change to IOException on Write when disposed/closed instead of timeoutexception?

I'll also look into the usecase BinaryReader if there's something better than can be done here.

Thanks for your input!

jcurl commented 8 years ago

You might want to try head (branch v2.x) now. MS implementation throws InvalidOperationException on Write() when closed, so that's what I do too. But I raise an IOException during the Write() operation if you Close() and ObjectDisposedException during the Write() if you Dispose().

Read() also throws ObjectDisposedException if disposed as that's consistent with Write() (it will still return the number of bytes in the buffer, or zero, if Close()'d).

jcurl commented 8 years ago

Resolved by https://github.com/jcurl/SerialPortStream/commit/a211272e043e70f340f1e6d6202e4b699656ac92

StarkillerObl commented 4 years ago

@jcurl Are You sure that Dispose() will always call Close()? I have a situation in which calling only Dispose() ends up with a blocked serial port (even after closing the process), but calling Close() and then Dispose() works perfectly fine (my program does this multiple times during execution, and it's predictable -> when only Dispose() is called it always won't work.