serial port threading hang

Question

I got the code below from a website,and this way of serial port reading is my only option because DataReceived event often doesn't work.but this code has a problem,if I close the application while transfering the application hangs forever,but I can't see why?neither freezing nor aborting the thread work.actually aborting the thread causes the program to crash.

public class CommPort
{
    SerialPort _serialPort;
    Thread _readThread;
    bool _keepReading;

    //begin Singleton pattern
    static readonly CommPort instance = new CommPort();

    // Explicit static constructor to tell C# compiler
    // not to mark type as beforefieldinit
    static CommPort()
    {
    }

    CommPort()
    {
        _serialPort = new SerialPort();
        _readThread = null;
        _keepReading = false;
    }

    public static CommPort Instance
    {
        get
        {
            return instance;
        }
    }
    //end Singleton pattern

    //begin Observer pattern
    public delegate void EventHandler(string param);
    public EventHandler StatusChanged;
    public EventHandler DataReceived;

    private void StartReading()
    {
        if (!_keepReading)
        {
            _keepReading = true;
            _readThread = new Thread(new ThreadStart(ReadPort));
            _readThread.Start();
        }
    }
    private void StopReading()
    {
        if (_keepReading)
        {
            _keepReading = false;
            _serialPort.Close();
            //_readThread.Join();   //block until exits
            _readThread.Abort();
            //_readThread = null;
        }
    }
    private void ReadPort()
    {
        while (_keepReading)
        {
            if (_serialPort.IsOpen)
            {
                byte[] readBuffer = new byte[_serialPort.ReadBufferSize + 1];
                try
                {
                    // If there are bytes available on the serial port,
                    // Read returns up to "count" bytes, but will not block (wait)
                    // for the remaining bytes. If there are no bytes available
                    // on the serial port, Read will block until at least one byte
                    // is available on the port, up until the ReadTimeout milliseconds
                    // have elapsed, at which time a TimeoutException will be thrown.
                    int count = _serialPort.Read(readBuffer, 0, _serialPort.ReadBufferSize);
                    String SerialIn = System.Text.Encoding.ASCII.GetString(readBuffer, 0, count);
                    DataReceived(SerialIn);
                }
                catch (TimeoutException)
                {
                }
            }
            else
            {
                TimeSpan waitTime = new TimeSpan(0, 0, 0, 0, 50);
                Thread.Sleep(waitTime);
            }
        }
    }


    /// <summary> Open the serial port with current settings. </summary>
    public void Open()
    {
        Close();

        try
        {
            _serialPort.PortName = Properties.Settings.Default.COMPort;
            _serialPort.BaudRate = Properties.Settings.Default.BPS;
            _serialPort.Parity = Properties.Settings.Default.Parity;
            _serialPort.DataBits = Properties.Settings.Default.DataBit;
            _serialPort.StopBits = Properties.Settings.Default.StopBit;
            _serialPort.Handshake = Properties.Settings.Default.HandShake;

            // Set the read/write timeouts
            _serialPort.ReadTimeout = 50;
            _serialPort.WriteTimeout = 50;

            _serialPort.Open();
            StartReading();
        }
        catch (IOException)
        {
            StatusChanged(String.Format("{0} does not exist", Properties.Settings.Default.COMPort));
        }
        catch (UnauthorizedAccessException)
        {
            StatusChanged(String.Format("{0} already in use", Properties.Settings.Default.COMPort));
        }
        catch (Exception ex)
        {
            StatusChanged(String.Format("{0}", ex.ToString()));
        }

        // Update the status
        if (_serialPort.IsOpen)
        {
            string p = _serialPort.Parity.ToString().Substring(0, 1); //First char
            string h = _serialPort.Handshake.ToString();
            if (_serialPort.Handshake == Handshake.None)
                h = "no handshake"; // more descriptive than "None"

            StatusChanged(String.Format("{0}: {1} bps, {2}{3}{4}, {5}",
            _serialPort.PortName, _serialPort.BaudRate,
            _serialPort.DataBits, p, (int)_serialPort.StopBits, h));
        }
        else
        {
            StatusChanged(String.Format("{0} already in use", Properties.Settings.Default.COMPort));
        }
    }

    /// <summary> Close the serial port. </summary>
    public void Close()
    {
        StopReading();
        StatusChanged("connection closed");
    }
    public bool IsOpen
    {
        get
        {
            return _serialPort.IsOpen;
        }
    }
}

Answer 1

When you close the Port in StopReading() it will cause an Exception in _serialPort.Read(...).

Not sure which one exactly but it's not a TimeOut. Your current code lets that escape and that's when your thread and your App are killed.

So add a catch(Exceptiopn ex) around the while loop in ReadPort().

Answer 2

Two issues at first glance.

First off, you have created a singleton class that exposes an event subscription. That's dangerous, because you should keep track of any of the subscribers. Otherwise they may be never released. That's probably what's going on in your case.

Secondly, when you manage something "IDisposable", you should take care about it. So, the SerialPort should be disposed inside a IDisposable implementation within your class. However, that won't make any sense in a singleton class.

The singleton pattern should be used only for centralized passive resources, and never to host "active" objects, events, etc.

Hope this helps.