[RESOLVED] Timer threading issues

[topshot]

TL;DR - why do module-declared Timers no longer fire when using the Start method from a thread in that module?

I am working with an API that has their sample code written in C# that is using threading and am having some issues getting to work well under certain cases for my needs. Because I'm under a NDA, I can't post full code but here is the barebones of their threading sample.

        private void Process_Click(object sender, EventArgs e)
        {          
            System.Windows.Forms.Control.CheckForIllegalCrossThreadCalls = false;
            XYZ.ManageRequest manageRequest = new XYZ.ManageRequest();
       
            pz.ManageRequest = manageRequest;

            ThreadStart entry1 = new ThreadStart(Run1);
            Thread process1 = new Thread(entry1);
            process1.IsBackground = true;
            process1.Start();
        }

        delegate void myDelegate1();
        void threadTrans()
        {          
           
        }
        void Run1()
        {
            myDelegate1 md1 = new myDelegate1(threadTrans);

            result = pz.ProcessTrans(); //THIS IS THE BLOCKING CALL
           
            this.Invoke(md1);

            if (result.Code == XYZ.ProcessResultCode.OK)
            {
                XYZ.ManageResponse manageResponse = pz.ManageResponse;
                if (manageResponse != null && manageResponse.ResultCode != null)
                {
                    //SET TEXT BOXES FROM ManageResponse PROPERTIES
                }
                else
                {
                    MessageBox.Show("Unknown error: pz.ManageResponse is null.");
                }
            }
            else if (result.Code == XYZ.ProcessResultCode.TimeOut)
            {
                MessageBox.Show("Action Timeout.");
            }
            else
            {
                MessageBox.Show(result.Msg, "Error Processing Payment", MessageBoxButtons.OK);
            }
            this.Focus();
        }

I need to do a lot more than just set some text boxes. My converted code, which is contained in a Module (mainfrm is the Public variable for my form instance):

    Public Sub SigCap()
        mainfrm.SigDisplayPB.Image = Nothing
        mainfrm.SigDisplayPB.Invalidate()
        Timer1.Stop()

        Dim manageRequest2 As New XYZ.ManageRequest()
        manageRequest2.TransType = ManageTransType.DOCAPTURE
        pz.ManageRequest = manageRequest2

        Dim entry1 As New ThreadStart(AddressOf RunSig)
        Dim process1 As New Thread(entry1)
        process1.IsBackground = True
        process1.Start()

    End Sub

    Delegate Sub myDelegateSig()
    Private Sub threadSig()

    End Sub

    Private Sub RunSig()
        Dim md1 As New myDelegateSig(AddressOf threadSig)

        Dim result2 As ProcessTransResult = pz.ProcessTrans()

        mainfrm.Invoke(md1)

        Select Case (result2.Code)
            Case ProcessResultCode.OK
                Dim mr As ManageResponse = pz.ManageResponse
                If mr.ResultTxt.ToUpper.Trim <> "OK" Then
                    MessageBox.Show("ERROR: " & mr.ResultTxt, "Error getting signature")
                    Exit Sub
                End If

                Dim manageRequest As New XYZ.ManageRequest()
                manageRequest.TransType = ManageTransType.GETCAPTURE
                manageRequest.SigSavePath = gTempImgLoc
                pz.ManageRequest = manageRequest
                Dim result As ProcessResult = pz.ProcessTrans() 'NEED TO ALSO MAKE A SECOND BLOCKING CALL TO RETRIEVE DATA
                Select Case (result.Code)
                    Case ProcessResultCode.OK
                        Dim mr2 As ManageResponse = pz.ManageResponse
                        Dim fullpath As String = Path.Combine(gTempImgLoc, mr2.SigFileName)
                        Dim fileext As String = "png"
                        gImgName = mr2.SigFileName & "_" & Trim(gInvId) & "." & fileext
                        If manageRequest.ConvertSigToPic(fullpath, fileext, Path.Combine(gTempImgLoc, gImgName)) = 0 Then
                            File.Delete(fullpath) 'remove CSV file
                            File.Copy(gTempImgLoc & gImgName, My.Application.Info.DirectoryPath & "\last." & fileext, True)
                            Using fs As New System.IO.FileStream(My.Application.Info.DirectoryPath & "\last." & fileext, IO.FileMode.Open, IO.FileAccess.Read)
                                mainfrm.SigDisplayPB.Image = System.Drawing.Image.FromStream(fs)
                            End Using
                            'mainfrm.Refresh() 'THIS LINE WOULD GET CROSSTHREAD ERROR

                           Thread.Sleep(1500)
                            Timer1.Interval = IdleTime
                            bgthread = New Thread(AddressOf MoveTempSigs)
                            bgthread.IsBackground = True
                            bgthread.Start() 'THIS THREAD WORKS

                            Timer1.Start()
                            TimerShowSig.Interval = 5000
                            TimerShowSig.Start() 'NEITHER TIMER STARTS - BOTH ARE DECLARED IN THE MODULE
                        Else
                            updateSigReqP("USERCANCEL")
                        End If

                    Case ProcessResultCode.ERROR
                        UpdMsg("Error converting signature: " & result.Msg, ErrorLevel.Low, False)

                    Case Else
                        UpdMsg("Action Timeout.", ErrorLevel.Low, False)

                End Select

            Case ProcessResultCode.ERROR
                UpdMsg("Error getting signature: " & result2.Msg, ErrorLevel.Low, False)

            Case Else
                UpdMsg("Action Timeout.", ErrorLevel.Low, False)

        End Select

        'mainfrm.Focus() 'THIS LINE WOULD GET CROSSTHREAD ERROR

    End Sub

I can live without the Focus and Refresh methods. My thought is that CheckForIllegalCrossThreadCalls = False allowed them to Focus in their sample. The more important question for me is how to properly call my Timers from within this thread. The form also has a Reset button that does Timer1.Start and it's not firing the Tick event anymore either so my program is dead in the water at that point. It as if the module's Timers have been totally disabled.

[dday9]

You will need to invoke either the control itself or the control that is its parent prior to doing anything with controls that sit on the UI thread. You should never have CheckForIllegalCrossThreadCalls set to False!!!

Here is an example of how to do something with a control that is on the UI thread from a separate thread:

Private Sub Form1_Load(ByVal sender As Object, ByVal e As EventArgs) Handles MyBase.Load
    Dim t As New Threading.Thread(AddressOf SomeReallyCoolThread)
    t.Start()
End Sub

Private Sub SomeReallyCoolThread()
    Me.Invoke(Sub()
                  TextBox1.Focus()
                  Timer1.Stop()
              End Sub)
End Sub

[Sitten Spynne]

There's a lot going on in your code, so I can't really pick out just the parts that need to change. You'll have to apply some thought, but I can expand on what dday9 said.

I want to re-iterate: setting CheckForIllegalCrossThreadCalls to false very bad in .NET. Even in sample code, it's a bad idea. The short story is making cross-thread calls can do anything from "works" to "nothing" to "crashes your application". On older versions of Windows, it could cause blue screens. That's why Visual Studio made special code in their debugger to detect it and let you know when it happens. This property is the switch to turn it back off. There is never a good reason to do so. This was a bad example.

Forms (and all controls) have thread affinity. That means you can't do anything with them from a thread other than the one that created them. We call that thread "the UI thread" since all UI work has to happen on it. If you aren't on that thread, doing something to that form can cause lots of problems. But. There are two safe things you can do to a form.

To ask if you're on the right thread, you can use the InvokeRequired property. If it's True, you are not on the right thread. This is safe no matter what thread calls it.

To do something to the form, you use the Invoke() method. It accepts a delegate that represents the code to execute, and will make sure that code gets scheduled and executed on the UI thread. This is safe, no matter what thread calls it.

Now, here's another problem you're likely having. If you're using the System.Windows.Forms.Timer class, that one behaves like a control. It has thread affinity. If it is created by a Form, it guarantees it will raise its Tick event on the UI thread. If it gets created on another thread, all bets are off and it makes no guarantees. If your timer's being created within a Module, that might mean it's being created on a special thread for static initialization, which isn't the UI thread and doesn't need further explaining.

It's safest, when working with multiple threads, to use System.Threading.Timer. You could also choose System.Timers.Timer. Yes, there are three things called "Timer". The problem with System.Timers.Timer is it ALSO has some code that tries to make guarantees about the thread it will operate on, so if you're not careful it can have the same problems as System.Windows.Forms.Timer. So my rule of thumb is "use System.Threading.Timer if threads are involved". It has a janky API, so I'll help explain it.

Below is a short example of a module and a form interacting with a threaded timer doing something. The module has a list of strings. The Form has a textbox. Every time the timer ticks (every second), a different string will be displayed. You need to write code similar to this for EACH thing you need to do with the form.

Public Module TimerLogic

    Private _timer As System.Threading.Timer

    Private _lineIndex As Integer = 0
    Private _lines() As String = {
        "This is the first message.",
        "It's not really exciting.",
        "Bye now.",
        "THE END"
    }

    Public Sub ModuleStart(ByVal theForm As Form1)
        ' Read the documentation but the order of parameters is:
        '   1) The method to call each 'tick'.
        '   2) An optional object for context. I'm using the form as the context.
        '      You'll see why later.
        '   3) The 'due time' before the first tick.
        '   4) The 'period', or how long between each tick after the first.
        ' I don't have to call Start() because a due time is specified.
        _timer = New Threading.Timer(AddressOf TimerRun, theForm, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(1))
    End Sub

    Private Sub TimerRun(ByVal state As Object)
        ' Get the form out of the state parameter.
        Dim theForm = CType(state, Form1)

        ' Get the string and update the form, then move to the next string.
        Dim message = _lines(_lineIndex)
        theForm.UpdateTextSafe(message)
        _lineIndex += 1
        If _lineIndex >= _lines.Length Then
            _lineIndex = 0
        End If
    End Sub

End Module

Public Class Form1

    ' I like to make public methods for the things my program needs to do to my form. I like
    ' to name them "Safe" when they are safe for multi-threaded operations.
    Public Sub UpdateTextSafe(ByVal message As String)
        If Me.InvokeRequired Then
            ' This happens when the method is called on the wrong thread. Use Invoke
            ' to re-call on the right thread.
            Me.Invoke(Sub() UpdateTextSafe(message))
        Else
            ' This happens when the method is called on the right thread. Update the
            ' text box.
            TextBox1.Text = message
        End If
    End Sub

    ' Start the timer when the form is shown.
    Protected Overrides Sub OnShown(e As EventArgs)
        MyBase.OnShown(e)

        TimerLogic.ModuleStart(Me)
    End Sub

End Class

There might be some better ways to structure your code. Keep in mind their example was intended to make something you could paste quickly and see work. It doesn't look like it was meant to suggest a particular architecture.

[topshot]

I want to re-iterate: setting CheckForIllegalCrossThreadCalls to false very bad in .NET. Even in sample code, it's a bad idea.

I'm well aware of that which is why I didn't put it in my code.

There might be some better ways to structure your code. Keep in mind their example was intended to make something you could paste quickly and see work. It doesn't look like it was meant to suggest a particular architecture.

You'd think they would do good practice but the use of CheckForIllegalCrossThreadCalls made me immediately suspect.

My other issue is I'm having to integrate this into a project that also was not done as well as it should have been. Needs to be in production in a week. I'm thinking having a responsive UI all the time is going to wait in this case though I'll play with the other 2 timers. At least the threading works for what I need it for on startup where being responsive is important.

[Sitten Spynne]

Yeah, I can devil's advocate either way on the examples. One of my products was targeted towards what we called "engineer-developers", which meant, "People who aren't programmers by career but sometimes have to write code to get their job done." We found these people didn't want to study architecture or deal with complex constructs in code. If our examples were very complex, they'd usually end up calling our support. They paid for that support, but in general we figured the more support calls/emails we got the worse our documentation was.

What I found was in cases like this, giving an engineer the "dirty" code meant he was going to try and write his application based off of that code. Then we'd get a support call/email about 2 months later, when his application isn't working properly. Those were much more involved support incidents, and if they were an "important" enough customer we'd have to potentially help them rewrite the whole thing. So it was often worth doing it right and dealing with people who didn't understand the appropriate architecture than doing it wrong and letting customers fail closer to their deadlines. (Which sounds kind of like where you are.)

I think nothing really highlights how important understanding multithreading is in UI code. I remember when I first started WinForms in 2003 or so I thought it was for advanced applications only. Now I can't imagine writing anything but the most basic tutorial app without getting something asynchronous involved.

When you get the time, I strongly suggest looking into the BackgroundWorker class first. It's probably the most user-friendly way to get asynchronous code working in .NET. With some mastery the Task API is powerful, but it has enough gotchas I've quit recommending it to people who haven't already had some experience with asynchronous programming.

[topshot]

I've used the BGW several times for other projects I've done on my own. Never had to involve a timer but I could easy enough I suppose. Maybe I should try that route.

I'm curious about something. Their pattern of (using my VB equivalent):

    Delegate Sub myDelegateSig()
    Private Sub threadSig()

    End Sub

    Private Sub RunSig()
        Dim md1 As New myDelegateSig(AddressOf threadSig)
        mainfrm.Invoke(md1)

        ...
    End Sub

which looks really ugly is just a long form for this (mostly following jmcilhinney's pattern), correct?

    Private Delegate Sub RunSigInvoker()

    Private Sub RunSig()

        mainfrm.Invoke(New RunSigInvoker(AddressOf RunSig))

        ...
    End Sub

[Sitten Spynne]

Yes, it's a longer syntax, and honestly using a Delegate signature at all is a bit archaic these days.

The next-best thing is to use the already-defined Delegate Action:

mainfrm.Invoke(New Action(AddressOf RunSig))

But more modern VB would just use a Lambda:

mainfrm.Invoke(Sub() RunSig())

[topshot]

Thanks.

BTW, using a BGW worked like a champ for the timers as far as I can tell. I went ahead and converted the other 2 threads to BGWs, too, just for consistency.