0

So I've recently come across a what I think is a deadlock that only happens sometimes. After I close my form the app just hangs. I'm using SerialPorts for communication. I've read this answer and this answer which both suggest I use BeginInvoke() instead of Invoke() for most methods unless I need to wait for a control to update.

  1. I don't know if this makes sense but I'm trying to figure out how to make the bug happen every time I close the form, so that if its resolved by using BeginInvoke I'm sure its gone. My code to close the port is
private void closeDevicePort()
{
    if (devicePort != null)
    {
        try
        {
            setMode("D0");
            toggleAllBtns();
            devicePort.Close();
            devicePort = null; //sometimes doesn't get past here
            Console.WriteLine("closing++++ here");
        }
        catch (Exception ex) {
            Console.WriteLine(ex);
        }
        
        //devicePort = null;

    }
}

which gets called through a GUIForm_FormClosed event on FormClosed. How do I make this happen every time to test that changing to BeginInvoke() actually works for my winforms app?

Also here is my DataReceivedHandler where I set the devicePort (which I didn't think was making anything explicitly multi-threaded)

private void DataReceivedHandler(object sender, SerialDataReceivedEventArgs e)
{
    SerialPort serialPort1 = (SerialPort)sender;
    if (serialPort1.IsOpen)
    {
        try
        {
            string message = serialPort1.ReadLine();
            if (devicePort == null)
            {
                devicePort = serialPort1;
            }
        }catch(Exception ex)
        {
        Console.WriteLine(ex);
        }
    }
}
  1. I have a plotData() function which plots data in realtime. Is using BeginInvoke() instead of Invoke() fine here too?
private void DataReceivedHandler(object sender, SerialDataReceivedEventArgs e)
{
    SerialPort serialPort1 = (SerialPort)sender;
    
    if (serialPort1.IsOpen)
    {
        try
        {
            string message = serialPort1.ReadLine();
            Console.WriteLine(message);

            if (message.Substring(0, 1) == "!")
            {
                oldTime = newTime;
                //checkPortsTimer.Stop();
                //checkPortsTimer.Interval = 1500;
                //checkPortsTimer.Start();
            }
            string flag = message.Substring(1, 1);
            string command = message.Substring(2, 1);
            string package = message.Substring(3);

            switch (flag)
            {
                
                case "S": 

                    switch (command)
                    {

                        case "C": //ready mode
                            setMode("SC");
                            if (devicePort == null)
                            {
                                addDevicePort(serialPort1);
                            }
                            break;
                    }
                    break;
                case "D": //data stream
                    float[] data = Array.ConvertAll(package.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries), float.Parse);
                    txtData_Update(data);
                    plotData(data);
                    break;
                default:
                    Console.WriteLine("not a recognised command. Try Again");
                    break;

            }
        }
        catch (Exception ex)
        {
            Console.WriteLine("in data received exception handler");
            Console.WriteLine(ex);

        }

    }
}
kenshima
  • 401
  • 4
  • 12
  • 2
    What is that you want to `BeginInvoke()`? Is that `devicePort.Close();` call that doesn't return/hangs? In this case, that's the problem you have to solve. Are you receiving/sending data in a worker Thread? Where is `devicePort` created and what is it? A SerialPort? You've tagged this question `multithreading`, but it's not clear what kind of threading this is and what kind of operations it implies. – Jimi Mar 23 '21 at 19:31
  • Hi @jim so `devicePort.Close()` hangs. Yes its a SerialPort. I want to `BeginInvoke()` a bunch of updates to GUI text, labels etc which should be fine to use `BeginInvoke()`. My concern is with a realtime plotting function which I'll add now. I haven't explicitly multi-threaded the app but winforms apps are implicitly multi-threaded no? Hence the need for `Invoke` in the first place. – kenshima Mar 23 '21 at 19:41
  • A WinForms app is *explicitly* single-threaded. You don't have any form of threading setup yet and you're worrying about possible threading issues? If/when you have some form of threading, it will be determined what kind of method to use to updated the UI, when data comes from worker Threads, that is. Probably neither `BeginInvoke()` or `Invoke()`. – Jimi Mar 23 '21 at 19:46
  • then I'm missing something. How does one get `System.InvalidOperationException: Cross-thread operation not valid: Control 'realtimePlot' accessed from a thread other than the thread it was created on.` then? By this I mean by taking out the `Invoke()` calls. – kenshima Mar 23 '21 at 19:50
  • Because `plotData()` is called from a worker Thread? You said *I haven't explicitly multi-threaded the app*, but you have methods running in worker Threads, apparently. Not clear how `devicePort` is involved here, it just appears in a snippet that is then not exactly related to the other. What is calling `plotData()`? Did you start a Thread at some point? In what Thread is `devicePort` created? Note that you can pass the current `SynchronizationContext` to a Thread (or you can capture it), or a `Progress` delegate. Or, don't start a Thread, run a Task (use the async/await if possible) etc. – Jimi Mar 23 '21 at 19:55
  • ok so I have a `DataReceivedHandler(object sender, SerialDataReceivedEventArgs e)` in which I set the devicePort to the SerialPort. I was under the impression that you have to call something like `Thread thread1 = new Thread(ThreadWork.DoWork);` to create threads. – kenshima Mar 23 '21 at 20:05
  • The `DataReceived` event is not raised on the UI Thread. To marshal the data used to updated the UI, you can follow the (somewhat ancient) `InvokeRequired/Invoke` pattern, as you have found it. `Invoke()` is synchronous. This can *mitigate* synchronization issues. But the issue that is synchronous remains. Calling `BeginInvoke()` there, may solve the usual port `Close()` hang issues, but may cause others to appear (UI updates synchronization). Unwire the event handler, wait a second, then just close the Form -- Don't do this: `devicePort = serialPort1;`. – Jimi Mar 23 '21 at 20:31
  • If you want to try the `SynchronizationContext` + `SendOrPostCallback` pattern (so you can more directly control the delegate and how UI updates are perform without blindly relying on Invoking Controls), I've posted something like this yesterday [here](https://stackoverflow.com/a/66755883/7444103) -- As a note, there's also the hack of starting a secondary Thread to call `Close()`, then call back to the UI Thread to close the Form after a *timeout*. I think that what has been described before is enough. – Jimi Mar 23 '21 at 20:40

0 Answers0