Search code examples
vb.netmultithreadingasynchronoustimermarshalling

VB.NET Trying to modify a generic Invoke method to a generic BeginInvoke method, having unexpected problems


VB.NET 2010, .NET 4

Hello,

I have been using a pretty slick generic invoke method for UI updating from background threads. I forget where I copied it from (converted it to VB.NET from C#), but here it is:

Public Sub InvokeControl(Of T As Control)(ByVal Control As t, ByVal Action As Action(Of t))
    If Control.InvokeRequired Then
        Try
            Control.Invoke(New Action(Of T, Action(Of T))(AddressOf InvokeControl), New Object() {Control, Action})
        Catch ex As Exception
        End Try
    Else
        Action(Control)
    End If
End Sub

Now, I want to modify this to make a function that returns Nothing if no invoke was required (or an exception was thrown) or the IAsyncResult returned from BeginInvoke if invoke was required. Here's what I have:

Public Function InvokeControl(Of T As Control)(ByVal Control As t, ByVal Action As Action(Of t)) As IAsyncResult
    If Control.InvokeRequired Then
        Try
            Return Control.BeginInvoke(New Action(Of T, Action(Of T))(AddressOf InvokeControl), New Object() {Control, Action})
        Catch ex As Exception
            Return Nothing
        End Try
    Else
        Action(Control)
        Return Nothing
    End If
End Function

I wanted to do this primarily to avoid blocking. The problem is that I now get errors when making calls such as this:

InvokeControl(SomeTextBox, Sub(x) x.Text = "Some text")

This worked fine with the original Invoke (rather than BeginInvoke) method. Now I get a "Object reference not set to an instance of an object" exception. If I put a watch on SomeTextBox, it says

SomeTextBox {Text = (Text) threw an exception of type Microsoft.VisualStudio.Debugger.Runtime.CrossThreadMessagingException.}

It might be relevant that such InvokeControl calls come from within a System.Timers.Timer's Elapsed event. Its Interval is 500ms, which should be more than long enough for the UI updates to complete (if that matters). What is going on?

Thanks in advance for the help!

Edit: More details

Here is my System.Timer.Timer's Elapsed handler:

Private Sub MasterTimer_Elapsed(ByVal sender As Object, ByVal e As System.Timers.ElapsedEventArgs) Handles MasterTimer.Elapsed
    MasterTimer.Enabled = False
    If Not MasterTimer.Interval = My.Settings.TimingMasterTimerInterval Then
        MasterTimer.Interval = My.Settings.TimingMasterTimerInterval
        NewEventLogEntry("The master timer's interval has been changed to " & MasterTimer.Interval.ToString & " milliseconds.")
    End If
    InvokeControl(TimerPictureBox, Sub(x) x.Toggle(True))
    ReadFromDevices()
    UpdateIndicators()
    'This block is not executing when the error is thrown
    If Mode > RunMode.NotRunning Then
        UpdateProcessTime()
        UpdateRemainingTime()
        UpdateStatusTime()
    End If
    'This block is not executing when the error is thrown
    If Mode = RunMode.Running Then
        CheckMillerCurrent()
        CheckTolerances()
    End If

    MasterTimer.Enabled = True
End Sub

Private Sub ReadFromDevices()
    For Each dev As Device In Devices
        Try
            If dev.GetType.Equals(GetType(Miller)) Then
                Dim devAsMiller As Miller = CType(dev, Miller)
                With devAsMiller
                    If .PowerOn.Enabled Then .PowerOn.Read()
                    If .CurrentRead.Enabled Then .CurrentRead.Read()
                    If .VoltageRead.Enabled Then .VoltageRead.Read()
                    If .Trigger.Enabled Then .Trigger.Read()
                    If .Shutter.Enabled Then .Shutter.Read()
                End With
            ElseIf dev.GetType.Equals(GetType(SubstrateBiasVoltage)) Then
                Dim devAsSubstrateBiasVoltage As SubstrateBiasVoltage = CType(dev, SubstrateBiasVoltage)
                With devAsSubstrateBiasVoltage
                    If .LambdaCurrentRead.Enabled Then .LambdaCurrentRead.Read()
                    If .LambdaVoltageRead.Enabled Then .LambdaVoltageRead.Read()
                    If .BiasResistor.Enabled Then .BiasResistor.Read()
                    If .Pinnacle.Enabled Then .Pinnacle.Read()
                End With
            Else
                If dev.Enabled Then dev.Read()
            End If
        Catch ex As Exception
            NewEventLogEntry("An error occurred while trying to read from a device.", ex, EventLogItem.Types.Warning)
        End Try
    Next
End Sub

Private Sub UpdateIndicators()
    Dim ObjLock As New Object
    SyncLock ObjLock
        With Devices
            InvokeControl(EmergencyStopPictureBox, Sub(x As DigitalPictureBox) x.Toggle(Mode > RunMode.NotRunning))

            InvokeControl(MillerCurrentIndicator, Sub(x) x.Text = .Miller1.CurrentRead.GetParsedValue.ToString)
            InvokeControl(MillerVoltageIndicator, Sub(x) x.Text = .Miller1.VoltageRead.GetParsedValue.ToString)
            With .SubstrateBiasVoltage
                InvokeControl(LambdaVoltageIndicator, Sub(x) x.Text = .LambdaVoltageRead.GetParsedValue.ToString)
                InvokeControl(LambdaCurrentIndicator, Sub(x) x.Text = .LambdaCurrentRead.GetParsedValue.ToString)
                InvokeControl(PinnacleVoltageIndicator, Sub(x) x.Text = .Pinnacle.GetParsedValue.ToString)
                InvokeControl(PinnacleCurrentIndicator, Sub(x) x.Text = .Pinnacle.ReadCurrent.ToString)
            End With
            InvokeControl(HeaterPowerIndicator, Sub(x) x.Text = .HeaterPower.GetParsedValue.ToString)
            InvokeControl(ConvectronIndicator, Sub(x) x.Text = .Convectron.GetParsedValue.ToString)
            If .Baratron.GetParsedValue > 200 Then
                InvokeControl(BaratronIndicator, Sub(x) x.Text = "OFF")
            Else
                InvokeControl(BaratronIndicator, Sub(x) x.Text = .Baratron.GetParsedValue.ToString)
            End If
            If .Ion.GetParsedValue > 0.01 Then
                InvokeControl(IonIndicator, Sub(x) x.Text = "OFF")
            Else
                InvokeControl(IonIndicator, Sub(x) x.Text = .Ion.GetParsedValue.ToString)
            End If
            InvokeControl(ArgonFlowRateIndicator, Sub(x) x.Text = .ArgonFlowRate.GetParsedValue.ToString)
            InvokeControl(NitrogenFlowRateIndicator, Sub(x) x.Text = .NitrogenFlowRate.GetParsedValue.ToString)
            InvokeControl(GateValvePositionIndicator, Sub(x) x.Text = .GateValvePosition.GetParsedValue.ToString)

            InvokeControl(RoughingPumpPowerOnIndicator, Sub(x As PowerButton) x.IsOn = .RoughingPumpPowerOn.Value = Power.On)

            ToggleImageList(.Miller1.CurrentRead.ImageList, .Miller1.CurrentRead.GetParsedValue > My.Settings.MinimumMillerCurrent)
            ToggleImageList(.Miller1.Trigger.ImageList, .Miller1.Trigger.GetParsedValue = Power.On)
            ToggleImageList(.HeaterPower.ImageList, .HeaterPower.Value > 0)
            With .SubstrateBiasVoltage
                ToggleImageList(.LambdaVoltageRead.ImageList, .LambdaVoltageRead.GetParsedValue > 0 And .BiasResistor.GetParsedValue = BiasResistor.Lambda)
                ToggleImageList(.Pinnacle.ImageList, .Pinnacle.GetParsedValue > 10 And .BiasResistor.GetParsedValue = BiasResistor.Pinnacle)
            End With
            ToggleImageList(.ArgonValveOpen.ImageList, .ArgonValveOpen.Value = Valve.Open)
            ToggleImageList(.NitrogenValveOpen.ImageList, .NitrogenValveOpen.Value = Valve.Open)
            ToggleImageList(.RoughingPumpValveOpen.ImageList, .RoughingPumpValveOpen.Value = Valve.Open)
            ToggleImageList(.SlowPumpDownValve.ImageList, .SlowPumpDownValve.Value = Valve.Open)
            ToggleImageList(.RotationPowerOn.ImageList, .RotationPowerOn.Value = Power.On)
            ToggleImageList(.WaterMonitor1.ImageList, .WaterMonitor1.Value = Power.On And .WaterMonitor2.Value = Power.On)
            ToggleImageList(.GateValvePosition.ImageList, .GateValvePosition.SetValue > 0)
        End With
    End SyncLock
End Sub

Private Sub ToggleImageList(ByRef ImageList As ImageList, ByVal IsOn As Boolean)
    For Each img As OnOffPictureBox In ImageList
        SafeInvokeControl(img, Sub(x As OnOffPictureBox) x.Toggle(IsOn))
    Next
End Sub

I hope that's not TMI, but hopefully it'll help spot what going wrong.

Also, with the watch on one of the textboxes and some breakpoints, I found that the error is somehow magically thrown after ReadFromDevices but before UpdateIndicators. By that, I mean that a breakpoint at the very end of ReadFromDevices shows the textboxes haven't thrown the error, but a breakpoint at the beginning of UpdateIndicators (before any InvokeControl calls have been made) shows that they have...


Solution

  • It is difficult to use debugging to catch the exception as it will occur on any one of multiple PostMessage calls to the UI message pump (caused by InvokeControl & the BeginInvoke calls). Visual Studio will have a hard time breaking on the exception. This is possibly why it appears that the exception is "magically" being thrown.

    Your issue lies not in your implementation of InvokeControl but in the UpdateIndicators method. It is because of the use of the With statement and the asynchronous UI thread calls like this:

    With Devices
        ...
        InvokeControl(MillerCurrentIndicator, Sub(x) x.Text = .Miller1.CurrentRead.GetParsedValue.ToString)
        ... 
     End With
    

    Because the Sub(x) code is being executed on the UI thread by posting a message on the UI thread it is extremely likely that the calling code on the current thread will have completed before the UI thread has been executed.

    The problem is with the underlying implementation of the Visual Basic With statement. In essence, the compiler creates an anonymous local variable for the With statement that gets set to Nothing at the End With statement.

    As an example, if you have this code:

    Dim p As New Person
    With p
        .Name = "James"
        .Age = 40
    End With
    

    The Visual Basic compiler turns this into:

    Dim p As New Person
    Dim VB$t_ref$L0 As Person = p
    VB$t_ref$L0.Name = "James"
    VB$t_ref$L0.Age = 40
    VB$t_ref$L0 = Nothing
    

    So, in your case, when the UI thread code is executed this anonymous local variable is now Nothing and you get your "Object reference not set to an instance of an object" exception.

    Your code essentially is equivalent to this:

    Dim VB$t_ref$L0 = Devices
    Dim action = new Action(Sub(x) x.Text = VB$t_ref$L0.Miller1.CurrentRead.GetParsedValue.ToString);
    VB$t_ref$L0 = Nothing
    action(MillerCurrentIndicator);
    

    By the time that the action is called the VB$t_ref$L0 variable is already set to Nothing and whammo!

    The answer is not to use a With statement. They are bad.


    That should be your answer, but there are a number of other issues in your code though that you should look at too.

    Your SyncLock code is using a local locking variable which essentially renders the lock useless. So don't do this:

    Private Sub UpdateIndicators()
        Dim ObjLock As New Object
        SyncLock ObjLock
        With Devices
            ...
        End With
        End SyncLock
    End Sub
    

    Do this instead:

    Private ObjLock As New Object
    Private Sub UpdateIndicators()
        SyncLock ObjLock
        With Devices
            ...
        End With
        End SyncLock
    End Sub
    

    All of the calls to InvokeControl in the UpdateIndicators method are making it difficult to debug your code. Reducing all of these calls to a single call should help you immensely. Try this instead:

    Private Sub UpdateIndicators()
        SyncLock ObjLock
            InvokeControl(Me, AddressOf UpdateIndicators)
        End SyncLock
    End Sub
    
    Private Sub UpdateIndicators(ByVal form As ControlInvokeForm)
        With Devices
            EmergencyStopPictureBox.Toggle(Mode > RunMode.NotRunning)
            MillerCurrentIndicator.Text = .Miller1.CurrentRead.GetParsedValue.ToString
            ...
            ToggleImageList(.GateValvePosition.ImageList, .GateValvePosition.SetValue > 0)
        End With
    End Sub
    

    Obviously you need to remove the With Devices code to make these work.

    There are a number of issues with the following type of code:

    If .Ion.GetParsedValue > 0.01 Then
        InvokeControl(IonIndicator, Sub(x) x.Text = "OFF")
    Else
        InvokeControl(IonIndicator, Sub(x) x.Text = .Ion.GetParsedValue.ToString)
    End If
    

    The value of .Ion.GetParsedValue may have changed between the condition being evaluated and the Else statement being executed. This is compounded because the condition on the If statement is evaluated on the current thread, but the Else statement is executed on the UI thread so the delay could be great. Also, if the .Ion. class is not thread-safe you are exposing yourself to potential errors.

    Do this instead:

    Dim parsedIonValue = .Ion.GetParsedValue
    If parsedIonValue > 0.01 Then
        InvokeControl(IonIndicator, Sub(x) x.Text = "OFF")
    Else
        InvokeControl(IonIndicator, Sub(x) x.Text = parsedIonValue.ToString)
    End If
    

    (This also gets rid of your With issue.)

    Use AutoReset = True on your MasterTimer to automatically call Enabled = False when the event fires to avoid the (remote) possibility of race conditions.

    Your code also doesn't seem to be correct in that you are using With Devices in the UpdateIndicators method, but you have a For Each loop in the ReadFromDevices method. Devices then appears to be a collection, but the code in UpdateIndicators is using the Devices object as if it were a Device object. And it is calling .SubstrateBiasVoltage on the Devices object. So I'm not sure exactly what the object Devices is actually doing.

    In the ToggleImageList method you are passing the ImageList parameter is being passed ByRef but you are not changing the reference to the ImageList. It's better then to pass it in ByVal to avoid potential bugs.

    Also, rather than doing this:

    If dev.GetType.Equals(GetType(Miller)) Then
        Dim devAsMiller As Miller = CType(dev, Miller)
        With devAsMiller
    

    It would be cleaner to do this:

    Dim devAsMiller = TryCast(dev, Miller)
    If devAsMiller IsNot Nothing Then
        With devAsMiller
    

    I hope this doesn't seem like I'm sinking the boot in! Hopefully it is helpful.