"Safe handle has been closed" with SerialPort and a thread in C#

Question

Good afternoon everybody!

I have this threaded SerialPort wrapper that reads in a line from the serial port. Here is my thread's code.

protected void ReadData()
{
     SerialPort serialPort = null;
     try
     {
         serialPort = SetupSerialPort(_serialPortSettings);
         serialPort.Open();

         string data;
         while (serialPort.IsOpen)
         {
             try
             {

                 data = serialPort.ReadLine();
                 if (data.Length > 0)
                     ReceivedData(serialPort, new ReceivedDataEventArgs(data));

             }
             catch (TimeoutException)
             {
                 //  No action
             }
         }
     }
     catch (ThreadAbortException)
     {
         if (serialPort != null)
             serialPort.Close();
     }
}

when I call myThread.Abort(); I get an exception (with no line or reference to code) "Safe handle has been closed". Can anyone spot what I am doing wrong? Thanks.

By the way, I have a Start() and a Stop() that creates the thread and aborts the thread, respectfully.

Solution

I would suspect that it is because you are using Thread.Abort to end the thread - which is generally frowned upon. The thread behavior when you abort it is not predictable. Because of that, since the serial port is a wrapper over native code, there are native resources - represented by a SafeHandle in .NET - which get disposed of unexpectedly and so you get the Exception.

You can think about what happens with your thread like this:

• you start your thread
• you open the serial port (which allocates native resources and uses SafeHandle(s) to hold on to those resources)
• you start reading from the serial port
• then at some point (unexpected to your thread) you call Thread.Abort on it
• most likely the code in your thread is at that point trying to access the serial port (to read data)
• the thread gets killed and the serial port handle is destroyed implicitly
• you get an exception thrown from the code inside the ReadLine() function of the serial port because the handle it had is no longer valid

You really should use a different method for aborting the thread so that you get a proper chance to close and dispose of the serial port.

A proper way to close your thread could be implemented using a ManualResetEvent like this:

protected ManualResetEvent threadStop = new ManualResetEvent(false);

protected void ReadData()
{
     SerialPort serialPort = null;
     try
     {
         serialPort = SetupSerialPort(_serialPortSettings);
         serialPort.Open();

         string data;
         while (serialPort.IsOpen)
         {
             try
             {

                 data = serialPort.ReadLine();
                 if (data.Length > 0)
                     ReceivedData(serialPort, new ReceivedDataEventArgs(data));

             }
             catch (TimeoutException)
             {
                 //  No action
             }

             // WaitOne(0) tests whether the event was set and returns TRUE
             //   if it was set and FALSE otherwise.
             // The 0 tells the manual reset event to only check if it was set
             //   and return immediately, otherwise if the number is greater than
             //   0 it will wait for that many milliseconds for the event to be set
             //   and only then return - effectively blocking your thread for that
             //   period of time
             if (threadStop.WaitOne(0))
                 break;
         }
     }
     catch (Exception exc)
     {
         // you can do something here in case of an exception
         // but a ThreadAbortedException should't be thrown any more if you
         // stop using Thread.Abort and rely on the ManualResetEvent instead
     }
     finally
     {
         if (serialPort != null)
             serialPort.Close();
     }
}

protected void Stop()
{
    // Set the manual reset event to a "signaled" state --> will cause the
    //   WaitOne function to return TRUE
    threadStop.Set();
} 

Of course, when using the events method to stop the thread you have to be careful to include an event state check in all your long running loops or tasks. If you don't your thread may appear not to respond to your setting the event - until it gets out of the long-running loop, or task and gets a chance to "see" that the event has been set.

Comments

[Daniel A. White]

What would be the proper way to close my thread. The Stop()/Start() are used for when I reconfigure the port.

[Mike Dinescu]

There are a few ways you could do it and I've given an example using a ManualResetEvent which is a pretty common mechanism..

[Mike Dinescu]

You're welcome.. I hope you understand why Thread.Abort is not good practice. If you would like to find out more on the subject - there is a really great book about concurrency in Windows by Joe Duffy: bluebytesoftware.com/books/winconc/winconc_book_resources.ht‌​ml

[supercat]

I've found that unplugging USB-based serial ports when they're open tends to cause weird behaviors including "safe handle has been closed" exceptions. For devices which are supposed to operate as long as they're plugged in, but which may be unplugged at any time, the only remedy I've found to work consistently is to spawn a separate application to handle each device, and use some sort of pipe to bind such applications to the main app. Really horribly hokey, but I don't know what else to do.