views:

830

answers:

1

I'm running in to a problem that I was able to fix with Application.DoEvents, but don't want to leave that in because it might introduce all sorts of nasty problems.

Background: Our app is primarily a desktop app that makes many calls to a web service. We control everything but changes to the overall system design are not going to be seriously considered. One of those calls, Calculate, is used very often, and occasionally can take a few minutes to process all the data to return valid results.

Previously this call to Calculate was done synchronously and thus would block the UI leaving the user to wonder if the app had frozen or not, etc. I've successfully moved all the long wait calls to a BackgroundWorker and then made a simple Waiting screen that would cycle through a "Calculating..." animated message.

Now the problem arises when our UI code tries to call the calculate routine again prior to the first one finishing. I would get a "This BackgroundWorker is currently busy and cannot run multiple instances..." message. Which I thought should be controlled by the resetEvent.WaitOne() calls. It did not so I thought maybe another event controlling access to the entire routine would help, so I added the calcDoneEvent. This still did not fix the problem, but would cause it to block indefinitely on the 2nd call to Calculate's calcDoneEvent.WaitOne() call. Then on a whim I added the Application.DoEvents to the bottom of Calculate and viola, problem solved.

I don't want to leave that .DoEvents in there because I've read it can cause problems that later are very difficult to track down. Is there a better way to handle this situation?

Thanks in advance..

Private WithEvents CalculateBGW As New System.ComponentModel.BackgroundWorker
Dim resetEvent As New Threading.AutoResetEvent(False)
Dim calcDoneEvent As New Threading.AutoResetEvent(True)

Public Sub Calculate()

    calcDoneEvent.WaitOne() ' will wait if there is already a calculate running.'
    calcDoneEvent.Reset()

    ' setup variables for the background worker'

    CalculateBGW.RunWorkerAsync() ' Start the call to calculate'

    Dim nMsgState As Integer = 0
    ' will block until the backgorundWorker is done'
    Do While Not resetEvent.WaitOne(200) ' sleep for 200 miliseconds, then update the status window'
        Select Case nMsgState
            Case 1
                PleaseWait(True, vbNull, "Calculating.   ")
            Case 2
                PleaseWait(True, vbNull, "Calculating..  ")
            Case 3
                PleaseWait(True, vbNull, "Calculating... ")
            Case 4
                PleaseWait(True, vbNull, "Calculating....")
            Case Else
                PleaseWait(True, vbNull, "Calculating    ")
        End Select
        nMsgState = (nMsgState + 1) Mod 5
    Loop

    PleaseWait(False, vbNull) 'make sure the wait screen goes away'

    calcDoneEvent.Set() ' allow another calculate to proceed'
    Application.DoEvents() ' I hate using this here'
End Sub

Private Sub CalculateBGW_DoWork(ByVal sender As System.Object, _
    ByVal e As System.ComponentModel.DoWorkEventArgs) Handles CalculateBGW.DoWork
    Try
        'make WS Call, do data processing on it, can take a long time..'
        'No Catch inside the DoWork for BGW, or exception handling wont work right...'
        'Catch'
    Finally
        resetEvent.Set() 'unblock the main thread'
    End Try
End Sub

Private Sub CalculateBGW_RunWorkerCompleted(ByVal sender As Object, _
    ByVal e As System.ComponentModel.RunWorkerCompletedEventArgs) Handles CalculateBGW.RunWorkerCompleted

    'If an error occurs we must check e.Error prior to touching e.Result, or the BGW' 
    'will possibly "eat" the exception for breakfast (I hear theyre tasty w/ jam)'
    If Not (e.Error Is Nothing) Then

        'If a Web Exception timeout, retry the call'
        If TypeOf e.Error Is System.Net.WebException And _
            e.Error.Message = "The operation has timed out" And _
            intRetryCount < intRetryMax Then

            ' Code for checking retry times, increasing timeout, then possibly recalling the BGW'

            resetEvent.Reset()
            CalculateBGW.RunWorkerAsync() 'restart the call to the WS'
        Else
            Throw e.Error ' after intRetryMax times, go ahead and throw the error up higher'
        End If
    Else
        Try

            'normal completion stuff'

        Catch ex As Exception
            Throw
        End Try
    End If

End Sub
+2  A: 

You declared:

Private WithEvents CalculateBGW As New System.ComponentModel.BackgroundWorker
Dim resetEvent As New Threading.AutoResetEvent(False)
Dim calcDoneEvent As New Threading.AutoResetEvent(True)

as private fields of the containing class. Notice that this way, all calls to RunWorkerAsync() are referred to the same object instance of the BackgroundWorker class (that is, to the same object). That is why it is "busy". This code is built to hold only one BackgroundWorker at a given time.

If you mean to allow the UI code to call the Calculate() method whenever it needs to, you should declare CalculateBGW as a local variable within the Calculate() method, thus creating a new instance of the BackgroundWorker class with every call (and they will run asynchronosly). This means you'll have to add and remove the event handlers inside Calculate() as well, using AddHandler and RemoveHandler.

There are several approaches to updating the UI on the progress, but it is suggested to use the BackgroundWorker.ProgressChanged event and BackgroundWorker.ReportProgress method.

Use the BackgroundWorker.RunWorkerCompleted event as a callback trigger, reporting the UI that the calculation is completed, thus triggering the needed code to represent the result. This approach eliminates the need to maintain a thread looping around bossing the calculation thread - thereby eliminating the need for DoEvents(). It lets the calculation thread inform its boss when its done working, instead of having the boss checking the worker's status and going to sleep over and over.

M.A. Hanin
I think you just might be on to something there. If it didn't take so long to re-open the IDE on this ancient computer here I would try it right now, but instead it will wait until the morning..
Josh W.