Thread is stuck when executing SerialPort.Close()

Question

I am facing a problem that a thread in my application runs into a deadlock when I want to close the serial port of a gsm terminal. This problem is well known here and here but all advices in these threads didn't help me.

/// <summary>
///     Closes the COM port, prevents reading and writing
/// </summary>
public void Stop()
{
    Debug.WriteLine("stop called");
    var block = true;
    var bgw = new BackgroundWorker
    {
        WorkerReportsProgress = false,
        WorkerSupportsCancellation = false,
    };

    bgw.DoWork += (s, e) =>
    {
        if (!CanAccessPort())
            return;

        try
        {
            _serialPort.DataReceived -= Read;
            GC.ReRegisterForFinalize(_serialPort.BaseStream);
            _serialPort.Close();
            _isOpen = false;

        }
        catch (Exception ex)
        {
            throw new Exception(PORTERROR, ex);
        }
    };

    bgw.RunWorkerCompleted += (s, e) =>
    {
        Debug.WriteLine("block is set to false =)");
        block = false;
    };

    bgw.RunWorkerAsync();
    while (block)
        Thread.Sleep(250);
}

The code above runs forever when _serialPort.Close() is executed. As a recommended advice I read about running the close operation in a separate thread. I tried with BackgroundWorker and Thread classes but nothing did work. Using AutoResetEvent as suggested in another thread did not work either. Before closing the port I am sending some commands to it and receive several results but it won't close. When I run a simple command line program that starts the port, reads data and tries to close it, everything works and even without threads.

What could cause the deadlock? I am not doing any GUI related stuff as mentioned in almost all other answers.

DataReceived event handler code here:

/// <summary>
///     Reads input from the COM interface and invokes the corresponding event depending on the input
/// </summary>
private void Read(object sender, SerialDataReceivedEventArgs e)
{
    var buffer = new char[1024];
    var counter = 0;
    _keepRunning = true;
     if (_timeout == null)
     {
        // timeout must be at least 3 seconds because when sending a sms to the terminal the receive notification (+CSDI) can be lost with less timeout 
        _timeout = new Timer(3000);
        _timeout.Elapsed += (s, ev) =>
        {
            _keepRunning = false;
            _timeout.Stop();
        };
    }
    _timeout.Start();

    // cancel condition: no more new data for 3 seconds or "OK"/"ERROR" found within the result
    while (_keepRunning)
    {
        var toRead = _serialPort.BytesToRead;
        if (toRead == 0)
        {
            Thread.Sleep(100);
            continue;
        }
        _timeout.Stop();
        _timeout.Start();

        counter += _serialPort.Read(buffer, counter, toRead);

        // ok or error found in result string
        var tmp = new string(buffer).Replace("\0", "").Trim();
        if (tmp.EndsWith("OK") || tmp.EndsWith("ERROR"))
        {
            _timeout.Stop();
            _keepRunning = false;
        }
    }

    // remove empty array slots from the back
    var nullTerminalCounter = 0;
    for (var i = buffer.Length - 1; i != 0; i--)
    {
        if (buffer[i] == '\0')
        {
            nullTerminalCounter++;
            continue;
        }
        break;
    }
    Array.Resize(ref buffer, buffer.Length - nullTerminalCounter);
    var str = new String(buffer).Trim();

    // result must be something different than incoming messages (+CMTI: \"MT\", 25)
    if (!((str.StartsWith("+CMTI") || str.StartsWith("+CSDI")) && str.Length < 20))
    {
        // when an incoming message is received, it does not belong to the command issued, so result has not yet arrived, hence port is still blocked!
        _isBlocked = false;
        Debug.WriteLine("port is unblocked");
    }


    var args = new CommandReturnValueReceivedEventArgs
    {
        ResultString = str
    };
    OnCommandReturnValueReceived(this, args);
}

Answer 1

The issues I had went away if I ensured that Open and Close never were called at the same time by any thread. I did this by using a lock.

The essence is the following.

lock(_port)
    _port.Open(.....);

...

lock(_port)
    _port.Close(.....);

Answer 2

    if (toRead == 0)
    {
        Thread.Sleep(100);
        continue;
    }

This code is the basic source of the deadlock. The rule for SerialPort.Close() is that it can only close the serial port when none of the event handlers for SerialPort are active. Problem is, your DataReceived event handler is almost always active, waiting for data. This was not the intended use for the event. You are supposed to read whatever is available from the serial port, typically appending bytes to a buffer and get out. The event fires again when more bytes are available.

   while (_keepRunning)

Looks like you discovered this problem and tried to fix it with the Timer. That doesn't work either, in a very ratty way that's very difficult to debug. A bool variable is not a proper synchronization primitive, like ManualResetEvent. The while() loop will not see the _keepRunning variable turn to false when you target x86 and run the Release build of your program. Which enables the jitter optimizer, it is apt to store the variable in a cpu register. Declaring the variable volatile is required to suppress that optimization.

I suspect, but can't guarantee, that using volatile will solve your problem. And you probably want to set _keepRunning to false before calling Close() so you don't get the timeout delay.

A more structural fix is however indicated. Rewrite the DataReceived event handler so it never loops waiting for data. Given that this appears to be code to talk to a modem, a simple _serialPort.ReadLine() call should be all that's required.