Question Multithreaded app not handling all raised events

[Buho1]

This is my first excursion into multithreading.

Winforms VB.NET 4.0. I want to launch about 300 threads that calculate a result, and I want these 300 results displayed in the main form, results shown as they come in.

I initially wrote this, which makes sense to me:

Imports System.Threading

Public Class MyTask
    Public UID As String
    Public Num As Integer
    Public Result As Integer
    Public Event ThreadComplete(ByVal ThreadPayload As Tuple(Of String, Integer))

    Public Sub New(ByVal _uid As String, ByVal _num As Integer)
        UID = _uid
        Num = _num
        Result = 0
    End Sub

    Public Function Square(ByVal _num As Integer) As Integer
        Return _num * _num
    End Function

    Public Sub BackgroundProcess()
        Result = Square(Num)
        RaiseEvent ThreadComplete(New Tuple(Of String, Integer)(UID, Result))
    End Sub
End Class

Public Class Form1
    Private WithEvents oMyTask As MyTask

    Private Sub Form1_Load(sender As System.Object, e As System.EventArgs) Handles MyBase.Load
        For x As Integer = 0 To 20
            oMyTask = New MyTask("Task " + x.ToString(), x)
            Dim t As New Thread(AddressOf oMyTask.BackgroundProcess)
            t.Start()
        Next
    End Sub

    Private Sub displayResult(ByVal ThreadPayload As Tuple(Of String, Integer)) Handles oMyTask.ThreadComplete
        Me.ListBox1.Items.Add(ThreadPayload.Item1 + " - " + ThreadPayload.Item2.ToString())
    End Sub
End Class

It compiles, but the only result in ListBox1 is "Task 20 - 400". I modified the above code to launch just 2 threads, and when they are handled one at a time, launch one more, leaving off where a global counter was set.

Public Class Form1
    Private WithEvents oMyTask As MyTask
    Private threadLastMade As Integer

    Private Sub Form1_Load(sender As System.Object, e As System.EventArgs) Handles MyBase.Load
        For x As Integer = 0 To 1
            launchThread(x)
        Next
    End Sub

    Private Sub launchThread(ByVal x As Integer)
        oMyTask = New MyTask("Task " + x.ToString(), x)
        Dim t As New Thread(AddressOf oMyTask.BackgroundProcess)
        t.Start()
        threadLastMade = x
    End Sub

    Private Sub displayResult(ByVal ThreadPayload As Tuple(Of String, Integer)) Handles oMyTask.ThreadComplete
        Me.ListBox1.Items.Add(ThreadPayload.Item1 + " - " + ThreadPayload.Item2.ToString())
        If threadLastMade < 3 Then launchThread(threadLastMade + 1)
    End Sub
End Class

This worked better, ListBox1 displayed Task 1, Task 2, and Task 3, but still Task 0 was not shown - the initial thread I started.

Debugging this code, it is clear that

• the initial threads are started,
• the initial threads are processed and ThreadComplete is raised,
• but only the last of the initial threads is handled by displayResult!

From my initial For loop, all threads are started and processed and they all raise events, but only the last thread in that loop is handled! What's going on?! Am I not understanding something fundamental? The code above is reproducible - try it yourself.

 

[Herman]

I am not sure why you are trying to reinvent the wheel here, your "MyTask" class already pretty much exists in the form of a background worker. Also there is a good chance you will not be able to update any of your form's controls properly from that event as it is now, since the event is fired from another thread, so is executed the handler. You need to use a delegate and an invoke. Or use a System.ComponentModel.BackgroundWorker and it's all done for you.

 

[Buho1]

Thanks Herman. I wasn't aware of BackgroundWorker. I just played with it for a bit, and it's not apparent to me that it does what I want: run 300 tasks asynchronously and report the results as they come in. The sub that handles MyBgWorker.DoWork seems to do everything in one go. MyBgWorker.ProgressChanged event can be fired whenever I finish one of the 300, but there's no data payload to pass to the form (unless I'm mistaken).

I have years of experience as an ASP.NET programmer where multithreading is a bad idea; my current project needs a WinForm and multithreading is the perfect solution, but I'm underexperienced. I expected my need to be a fairly common one, but Google searches aren't yielding much about dealing with multiple, asynchronous threads and passing data from them to the main thread.

 

[Herman]

Well to run 300 tasks concurrently you would need 300 logical cores available. That won't happen. What you should do is split your 300 calculations into 4 or 8 subsets and run 4 or 8 threads running them. That will give you optimal performance without too much overhead. If you need to signal the main thread everytime one calculation is done, do so in the ProgressChanged handler.. Something like this:

Imports System.ComponentModel

Public Class Form1
    ' Number of total tasks to run
    Private intNumTasks As Integer = 10000
    ' Number of threads to use
    Private intNumThreads As Integer = 8
    ' UID
    Private _uid As String = "userid"
    ' Calculate the number of tasks each thread should complete
    Private intTasksPerThread As Integer = (intNumTasks \ intNumThreads) + 1

    ' Instanciate our BackgroundWorkers
    Private WithEvents bgWorker1, bgWorker2, bgWorker3, bgWorker4, _
                       bgWorker5, bgWorker6, bgWorker7, bgWorker8 As New BackgroundWorker With {.WorkerReportsProgress = True}

    Private Sub Form1_Shown(sender As System.Object, e As System.EventArgs) Handles Me.Shown
        ' Start each thread with their own subset of data. The argument passed here is accessible
        ' in the DoWork handler as e.Argument. In this case we pass an integer array containing
        ' the first and last data element to process for the series.
        bgWorker1.RunWorkerAsync(New Integer() {0, intTasksPerThread - 1})
        bgWorker2.RunWorkerAsync(New Integer() {intTasksPerThread, 2 * intTasksPerThread - 1})
        bgWorker3.RunWorkerAsync(New Integer() {2 * intTasksPerThread, 3 * intTasksPerThread - 1})
        bgWorker4.RunWorkerAsync(New Integer() {3 * intTasksPerThread, 4 * intTasksPerThread - 1})
        bgWorker5.RunWorkerAsync(New Integer() {4 * intTasksPerThread, 5 * intTasksPerThread - 1})
        bgWorker6.RunWorkerAsync(New Integer() {5 * intTasksPerThread, 6 * intTasksPerThread - 1})
        bgWorker7.RunWorkerAsync(New Integer() {6 * intTasksPerThread, 7 * intTasksPerThread - 1})
        bgWorker8.RunWorkerAsync(New Integer() {7 * intTasksPerThread, intNumTasks - 1})
    End Sub

    ' This handler is run when any of the bgWorker's .RunWorkerAsync method is called. It runs in its own thread.
    Private Sub bgWorkers_DoWork(sender As Object, e As DoWorkEventArgs) _
    Handles bgWorker1.DoWork, bgWorker2.DoWork, bgWorker3.DoWork, bgWorker4.DoWork, _
            bgWorker5.DoWork, bgWorker6.DoWork, bgWorker7.DoWork, bgWorker8.DoWork

        ' Get the arguments that were passed to the bgWorker cast back into an array.
        Dim intArgs As Integer() = CType(e.Argument, Integer())

        ' They are our first and last data elements to process
        Dim intStart As Integer = intArgs(0)
        Dim intEnd As Integer = intArgs(1)

        ' Process each element in the series, and fire the ReportProgress event for each. The .ReportProgress
        ' method accepts two arguments, the first one is an integer supposed to represent the percentage done,
        ' and the second is an object you can use to pass data back to the ProgressChanged handler.
        For i As Integer = intStart To intEnd
            CType(sender, BackgroundWorker).ReportProgress(0, New bgWorkerResult With {.UID = _uid, .Result = i * i})
        Next
    End Sub

    ' This handler runs everytime the .ReportProgress method is called. It runs on the UI thread.
    Private Sub bgWorkers_ProgressChanged(sender As Object, e As ProgressChangedEventArgs) _
        Handles bgWorker1.ProgressChanged, bgWorker2.ProgressChanged, bgWorker3.ProgressChanged, bgWorker4.ProgressChanged, _
                bgWorker5.ProgressChanged, bgWorker6.ProgressChanged, bgWorker7.ProgressChanged, bgWorker8.ProgressChanged

        ' Cast the .UserState object property back to our results type.
        Dim Result As bgWorkerResult = CType(e.UserState, bgWorkerResult)

        ' Add the passed data to the listbox. Ideally you might want to use a ListView instead, it's easier to sort properly.
        ListBox1.Items.Add(Result.UID & " - " & Result.Result.ToString)
    End Sub

    ' Our results datatype.
    Private Class bgWorkerResult
        Public UID As String
        Public Result As Long
    End Class

End Class

Of course I don't personally think that you need 8 threads to calculate 300 squares. A single BackgroundWorker would do it just fine, probably even faster. But if the actual task is more daunting this is how I would do it.

 

[Herman]

Also realize that with the amount of ProgressChanged events firing you will likely not see anything redraw until everything is finished, with the UI thread being busy handling those events constantly.

 

[JohnH]

From my initial For loop, all threads are started and processed and they all raise events, but only the last thread in that loop is handled! What's going on?!

You only have one WithEvents variable. One variable can only hold one object reference, so only the object it currently holds at the time it raises the event will be handled. To handle event for many dynamically created objects you need to use AddHandler statement, then you also need to use RemoveHandler statement before releasing the object.

If you want to create lots of short-lived threads, using the managed ThreadPool is much more efficient than creating threads yourself.

 

[JohnH]

I will show an example on how to easiest do this with the code you provided:

To create, add event handler and start the tasks:

        For x = 1 To 5
            Dim task As New MyTask("Task " + x.ToString(), x)
            AddHandler task.ThreadComplete, AddressOf DisplayResult
            Threading.ThreadPool.QueueUserWorkItem(New Threading.WaitCallback(AddressOf task.BackgroundProcess))
        Next

This will require your BackgroundProcess method to take the signature of WaitCallback delegate:

Public Sub BackgroundProcess(state As Object)

As for handling the event you need to use Control.Invoke to transfer back to UI thread. As mentioned you also need to RemoveHandler, and to do that you need the object reference. With a slight change in event signature you can get closer to coding conventions for events and add the 'sender' parameter:

Public Event ThreadComplete(sender As Object, threadPayload As Tuple(Of String, Integer))

When you raise the event you simply pass the Me object reference:

RaiseEvent ThreadComplete(Me, New Tuple(Of String, Integer)(UID, Result))

Finally the DisplayResult event handler can be modified like this:

    Private Sub DisplayResult(sender As Object, threadPayload As Tuple(Of String, Integer))
        Dim s = threadPayload.Item1 & " - " & threadPayload.Item2.ToString()
        Me.ListBox1.Invoke(New Threading.WaitCallback(AddressOf Me.ListBox1.Items.Add), s)
        RemoveHandler CType(sender, MyTask).ThreadComplete, AddressOf DisplayResult
    End Sub

Note that coding conventions for events is to provide only two parameters (sender As Object, e As EventArgs), where sender identifies the object that raised the event, and e provides additional event information. You can inherit EventArgs class to hold any additional info and use that as e parameter type.

 

[JohnH]

Two more notes about coding conventions.
In MyTask you have public fields, it is recommended fields are kept private and exposed as properties. In .Net 4 you can just as easily declare a property (with a hidden private backing field) like this:

Public Property UID As String
Public Property Num As Integer

This looks messy with all the _ chars:

    Public Sub New(ByVal _uid As String, ByVal _num As Integer)
        UID = _uid
        Num = _num
        Result = 0
    End Sub

You can and should use the same name for constructor parameters as the properties they are supposed to initialize (only with different cases according to naming conventions). In constructor the local parameter name has a higher scope and is used unless you qualify that you are referring to the property, like this:

    Public Sub New(ByVal uid As String, ByVal num As Integer)
        Me.UID = uid
        Me.Num = num
    End Sub

 

[Buho1]

You only have one WithEvents variable. One variable can only hold one object reference

Aah! That makes a lot of sense!

John, I took your Post #7 and applied it to my example and it worked fantastic! That's exactly what I was aiming for! I've applied it to my main project and it worked there, too, though I am struggling with a weird bug that is probably related to the multithreaded stuff. (I am dumping the results to a DataGridView. If I drag-select or Shift-Select some cells during multithreading operation, I get an index out of range exception at nowhere in particular that I can trace; sometimes it is in my code but at break, everything is within range; sometimes it is in the framework code, not mine.) I tried threadsafeing the OnClick event but that didn't help. Anyway, this little bug is worth having; it's a quick-and-dirty scanner for an immediate problem.

Yes, thank you about the public properties; I used public member variables to simplify and speed programming for this quick and dirty project.

Herman, thank you for your help. I need something more adaptable than your code. ThreadPool was perfect.

I understand multithreaded programming much more now! Thanks for your help, Herman and John!