ObjectDisposedException when closing SerialPort in .Net 2.0

Question

I have a C# windows forms application which communicates with a USB dongle via a COM port. I am using the SerialPort class in .Net 2.0 for communication, and the serial port object is open for the lifetime of the application. The application sends commands to the device and can also receive unsolicited data from the device.

My problem occurs when the form is closed - I get (randomly, unfortunately) an ObjectDisposedException when attempting to close the COM port. Here is the Windows stack trace:

System.ObjectDisposedException was unhandled


Message=Safe handle has been closed
  Source=System
  ObjectName=""
  StackTrace:
       at Microsoft.Win32.UnsafeNativeMethods.SetCommMask(SafeFileHandle hFile, Int32 dwEvtMask)
       at System.IO.Ports.SerialStream.Dispose(Boolean disposing)
       at System.IO.Ports.SerialStream.Finalize()
  InnerException: 

I have found posts from people with similar problems and have tried the workaround [here][1]

[1]: http://zachsaw.blogspot.com/2010/07/net-serialport-woes.html although that is for an IOException and did not stop the problem.

My Close() code is as follows:

        public void Close()
    {
        try
        {
            Console.WriteLine("******ComPort.Close - baseStream.Close*******");
            baseStream.Close();
        }
        catch (Exception ex)
        {
            Console.WriteLine("******ComPort.Close baseStream.Close raised exception: " + ex + "*******");
        }
        try
        {
            _onDataReceived = null;
            Console.WriteLine("******ComPort.Close - _serialPort.Close*******");
            _serialPort.Close();
        }
        catch (Exception ex)
        {
            Console.WriteLine("******ComPort.Close - _serialPort.Close raised exception: " + ex + "*******");
        }            
    }

My logging showed that execution never got beyond attempting to close the SerialPort's BaseStream (this is in the first try block), so I experimented with removing this line but the exception is still thrown periodically - the logging in the second try block appeared then the exception happened. Neither catch block catches the exception.

Any ideas?

UPDATE - adding full class:

    namespace My.Utilities
{
    public interface ISerialPortObserver
    {
        void SerialPortWriteException();
    }

    internal class ComPort : ISerialPort
    {
        private readonly ISerialPortObserver _observer;
        readonly SerialPort _serialPort;

        private DataReceivedDelegate _onDataReceived;
        public event DataReceivedDelegate OnDataReceived
        {
            add { lock (_dataReceivedLocker) { _onDataReceived += value; } }
            remove { lock (_dataReceivedLocker) { _onDataReceived -= value; } }            
        }

        private readonly object _dataReceivedLocker = new object();
        private readonly object _locker = new object();

        internal ComPort()
        {         
            _serialPort = new SerialPort { ReadTimeout = 10, WriteTimeout = 100, DtrEnable = true };
            _serialPort.DataReceived += DataReceived;
        }

        internal ComPort(ISerialPortObserver observer) : this()
        {
            _observer = observer;         
        }

        private void DataReceived(object sender, SerialDataReceivedEventArgs e)
        {
            DataReceivedDelegate temp = null;

            lock (_locker)
            {
                lock (_dataReceivedLocker)
                {
                    temp = _onDataReceived;
                }

                string dataReceived = string.Empty;
                var sp = (SerialPort) sender;

                try
                {
                    dataReceived = sp.ReadExisting();
                }
                catch (Exception ex)
                {
                    Logger.Log(TraceLevel.Error, "ComPort.DataReceived raised exception: " + ex);
                }

                if (null != temp && string.Empty != dataReceived)
                {
                    try
                    {
                        temp(dataReceived, TickProvider.GetTickCount());
                    }
                    catch (Exception ex)
                    {
                        Logger.Log(TraceLevel.Error, "ComPort.DataReceived raised exception calling handler: " + ex);
                    }
                }
            }
        }

        public string Port
        {
            set
            {
                try
                {
                    _serialPort.PortName = value;
                }
                catch (Exception ex)
                {
                    Logger.Log(TraceLevel.Error, "ComPort.Port raised exception: " + ex);
                }
            }
        }

        private System.IO.Stream comPortStream = null;
        public bool Open()
        {
            SetupSerialPortWithWorkaround();
            try
            {
                _serialPort.Open();
                comPortStream = _serialPort.BaseStream;
                return true;
            }
            catch (Exception ex)
            {
                Logger.Log(TraceLevel.Warning, "ComPort.Open raised exception: " + ex);
                return false;
            }
        }

        public bool IsOpen
        {
            get
            {
                SetupSerialPortWithWorkaround();
                try
                {
                    return _serialPort.IsOpen;
                }
                catch(Exception ex)
                {
                    Logger.Log(TraceLevel.Error, "ComPort.IsOpen raised exception: " + ex);
                }

                return false;
            }
        }

        internal virtual void SetupSerialPortWithWorkaround()
        {
            try
            {
                //http://zachsaw.blogspot.com/2010/07/net-serialport-woes.html
                // This class is meant to fix the problem in .Net that is causing the ObjectDisposedException.
                SerialPortFixer.Execute(_serialPort.PortName);
            }
            catch (Exception e)
            {
                Logger.Log(TraceLevel.Info, "Work around for .Net SerialPort object disposed exception failed with : " + e + " Will still attempt open port as normal");
            }
        }

        public void Close()
        {
            try
            {
                comPortStream.Close();
            }
            catch (Exception ex)
            {
                Logger.Log(TraceLevel.Error, "ComPortStream.Close raised exception: " + ex);
            }
            try
            {
                _onDataReceived = null;
                _serialPort.Close();
            }
            catch (Exception ex)
            {
                Logger.Log(TraceLevel.Error, "ComPort.Close raised exception: " + ex);
            }            
        }

        public void WriteData(string aData, DataReceivedDelegate handler)
        {
            try
            {
                OnDataReceived += handler;
                _serialPort.Write(aData + "\r\n");
            }
            catch (Exception ex)
            {
                Logger.Log(TraceLevel.Error, "ComPort.WriteData raised exception: " + ex);                

                if (null != _observer)
                {
                    _observer.SerialPortWriteException();
                }
            }
        }
    }    
}

Answer 1

Note: The current findings have only been tested on .NET Framework 4.0 32-bit on Windows 7, please feel free to comment if it works on other versions.

Edit: TL;DR: Here's the crux of the workaround. See below for explanation. Don't forget to use SerialPortFixer when opening the SerialPort as well. ILog is from log4net.

static readonly ILog s_Log = LogManager.GetType("SerialWorkaroundLogger");

static void SafeDisconnect(SerialPort port, Stream internalSerialStream)
{
    GC.SuppressFinalize(port);
    GC.SuppressFinalize(internalSerialStream);

    ShutdownEventLoopHandler(internalSerialStream);

    try
    {
        s_Log.DebugFormat("Disposing internal serial stream");
        internalSerialStream.Close();
    }
    catch (Exception ex)
    {
        s_Log.DebugFormat(
            "Exception in serial stream shutdown of port {0}: {1}", port.PortName, ex);
    }

    try
    {
        s_Log.DebugFormat("Disposing serial port");
        port.Close();
    }
    catch (Exception ex)
    {
        s_Log.DebugFormat("Exception in port {0} shutdown: {1}", port.PortName, ex);
    }
}

static void ShutdownEventLoopHandler(Stream internalSerialStream)
{
    try
    {
        s_Log.DebugFormat("Working around .NET SerialPort class Dispose bug");

        FieldInfo eventRunnerField = internalSerialStream.GetType()
            .GetField("eventRunner", BindingFlags.NonPublic | BindingFlags.Instance);

        if (eventRunnerField == null)
        {
            s_Log.WarnFormat(
                "Unable to find EventLoopRunner field. "
                + "SerialPort workaround failure. Application may crash after "
                + "disposing SerialPort unless .NET 1.1 unhandled exception "
                + "policy is enabled from the application's config file.");
        }
        else
        {
            object eventRunner = eventRunnerField.GetValue(internalSerialStream);
            Type eventRunnerType = eventRunner.GetType();

            FieldInfo endEventLoopFieldInfo = eventRunnerType.GetField(
                "endEventLoop", BindingFlags.Instance | BindingFlags.NonPublic);

            FieldInfo eventLoopEndedSignalFieldInfo = eventRunnerType.GetField(
                "eventLoopEndedSignal", BindingFlags.Instance | BindingFlags.NonPublic);

            FieldInfo waitCommEventWaitHandleFieldInfo = eventRunnerType.GetField(
                "waitCommEventWaitHandle", BindingFlags.Instance | BindingFlags.NonPublic);

            if (endEventLoopFieldInfo == null
                || eventLoopEndedSignalFieldInfo == null
                || waitCommEventWaitHandleFieldInfo == null)
            {
                s_Log.WarnFormat(
                    "Unable to find the EventLoopRunner internal wait handle or loop signal fields. "
                    + "SerialPort workaround failure. Application may crash after "
                    + "disposing SerialPort unless .NET 1.1 unhandled exception "
                    + "policy is enabled from the application's config file.");
            }
            else
            {
                s_Log.DebugFormat(
                    "Waiting for the SerialPort internal EventLoopRunner thread to finish...");

                var eventLoopEndedWaitHandle =
                    (WaitHandle)eventLoopEndedSignalFieldInfo.GetValue(eventRunner);
                var waitCommEventWaitHandle =
                    (ManualResetEvent)waitCommEventWaitHandleFieldInfo.GetValue(eventRunner);

                endEventLoopFieldInfo.SetValue(eventRunner, true);

                // Sometimes the event loop handler resets the wait handle
                // before exiting the loop and hangs (in case of USB disconnect)
                // In case it takes too long, brute-force it out of its wait by
                // setting the handle again.
                do
                {
                    waitCommEventWaitHandle.Set();
                } while (!eventLoopEndedWaitHandle.WaitOne(2000));

                s_Log.DebugFormat("Wait completed. Now it is safe to continue disposal.");
            }
        }
    }
    catch (Exception ex)
    {
        s_Log.ErrorFormat(
            "SerialPort workaround failure. Application may crash after "
            + "disposing SerialPort unless .NET 1.1 unhandled exception "
            + "policy is enabled from the application's config file: {0}",
            ex);
    }
}

I've wrestled with this for a couple of days in a recent project.

There are many different bugs (I've seen so far) with the .NET SerialPort class that lead to all of the headaches on the web.

1. The missing DCB struct flag here: http://zachsaw.blogspot.com/2010/07/net-serialport-woes.html This one is fixed by the SerialPortFixer class, credits go to the author for that one.

2. When a USB serial device is removed, when closing the SerialPortStream, the eventLoopRunner is asked to stop and SerialPort.IsOpen returns false. Upon disposal this property is checked and closing the internal serial stream is skipped, thus keeping the original handle open indefinitely (until the finalizer runs which leads to the next problem).

   The solution for this one is to manually close the internal serial stream. We can get its reference by SerialPort.BaseStream before the exception has happened or by reflection and getting the "internalSerialStream" field.

3. When a USB serial device is removed, closing the internal serial stream throws an exception and closes the internal handle without waiting for its eventLoopRunner thread to finish, causing an uncatchable ObjectDisposedException from the background event loop runner thread later on when the stream's finalizer runs (which oddly avoids throwing the exception but still fails to wait for the eventLoopRunner).

   Symptom of that here: https://connect.microsoft.com/VisualStudio/feedback/details/140018/serialport-crashes-after-disconnect-of-usb-com-port

   The solution is to manually ask the event loop runner to stop (via reflection) and waiting for it to finish before closing the internal serial stream.

4. Since Dispose throws exceptions, the finalizer is not suppressed. This is easily solvable:

   GC.SuppressFinalize(port); GC.SuppressFinalize(port.BaseStream);

Here's a class that wraps a serial port and fixes all these issues: http://pastebin.com/KmKEVzR8

With this workaround class, reverting to .NET 1.1 unhandled exception behavior is unnecessary and it works with excellent stability.

This is my first contribution, so please excuse me if I'm not doing it right. I hope it helps someone.

Answer 2

Yes, there's a flaw in the SerialPort class that makes this kind of crash possible. SerialPort starts up a thread when you call Open(). That thread watches for events on the port, that's how you get the DataReceived event for example. When you call the BaseStream.Close() or Close() or Dispose() method (they all do the same thing) then SerialPort only asks the thread to exit but doesn't wait for it to exit.

This causes all kinds of problems. One documented one, you're not supposed to Open() a port right after closing it. But the mishap here is when your program exits or garbage collects right after the Close() call. That runs the finalizer and it tries to close the handle as well. It is still open because the worker thread is still using it. A threading race is now possible, this isn't interlocked properly. The kaboom happens when the worker managed to close the handle and exit just before the finalizer thread tries to do the same. The exception is uncatchable because it happens in the finalizer thread, the CLR aborts the program.

Every version of .NET since 2.0 had small changes in the classes to work around SerialPort problems. By far the best thing to do if you're still on .NET 2.0 is to not actually call Close(). It happens automatically anyway, the finalizer takes care of it. Even if that doesn't happen for some reason (hard crash or program abort) then Windows ensures the port is closed.