Need a fix for my deadlocking issue with my parallel.foreach

Question

When running my code I seem to encounter deadlocks while trying to update a GUI element from within one of the parallel tasks.

I've tried surrounding the Output function with "Synclock me" to try to ensure that only one task is trying to update the control at a time.

Private Sub RunParallel(records as list(of DataRecord), ou as String)
    Dim ParallelOptions As New ParallelOptions
    ParallelOptions.MaxDegreeOfParallelism = 10
            Parallel.ForEach(records, ParallelOptions, Sub(myrecord)
                                                      ProcessRecord(myrecord, ou)
                                                  End Sub)
    Output("Done...." & vbCrLf)
End Sub

Private Sub ProcessRecord(ByVal record As DataRecord, ByVal ou As String)
    'Output($"BromcomID = {record("ID")}, Forename = {record("Forename")}{vbCrLf}")

    Dim ud As New UserDetails With {
        .EmployeeID = record("ID"),
        .SamAccountName = record("SamAccountName"),
        .GivenName = record("Forename"),
        .Surname = record("Surname")
    }
    If Not CreateUser(ou, ud) Then
        'Threading.Thread.Sleep(2000)
        ' Output($"Error creating {ud.EmployeeID}{vbCrLf}")
    End If
End Sub

Private Sub Output(ByVal s As String)
    SyncLock Me
        If Me.InvokeRequired Then

            Invoke(Sub()
                       Outbox.AppendText(s)
                       Outbox.SelectionStart = Len(Outbox.Text)
                       Outbox.ScrollToCaret()
                       Outbox.Select()
                   End Sub)
        Else
            Outbox.AppendText(s)
            Outbox.SelectionStart = Len(Outbox.Text)
            Outbox.ScrollToCaret()
            Outbox.Select()
        End If
    End SyncLock
End Sub

The code as supplied seems to run, but if I uncomment out the Output calls in the ProcessRecord() function, it hangs and never gets exits the Parallel.foreach

--- Update After playing around with suggestions and comments on here I still can't get it to work correctly. If I take out all of the output from ProcessRecord it seems to work correctly. However with the following code, it now seems to run each ProcessRecord sequentially (not 10 at a time as I intended), and then hangs after the last one.

    Output("Dispatching" & vbCrLf)
    Dim ParallelOptions As New ParallelOptions With {
        .MaxDegreeOfParallelism = 10
    }
    Parallel.ForEach(recordList, ParallelOptions, Sub(myrecord)
                                                      ProcessRecord(myrecord, ou)
                                                  End Sub)
    'For Each myrecord As DataRecord In recordList
    '    Task.Factory.StartNew(Sub() ProcessRecord(myrecord, ou))
    'Next
    Output("Done...." & vbCrLf)
End Sub

Private Sub ProcessRecord(ByVal record As DataRecord, ByVal ou As String)

    Dim ud As New UserDetails With {
        .EmployeeID = record("ID"),
        .SamAccountName = record("SamAccountName"),
        .GivenName = record("Forename"),
        .Surname = record("Surname"),
        .DisplayName = $"{record("Forename")} {record("Surname")} (Student)"}

    If Not CreateUser(ou, ud) Then
        ' Output($"Error creating {ud.EmployeeID}{vbCrLf}")
    End If
    Output($"BromcomID = {record("ID")}, Forename = {record("Forename")}{vbCrLf}")
End Sub

Private Sub Output(ByVal s As String)
    If Me.InvokeRequired Then
        Invoke(Sub()
                   Output(s)
               End Sub)
    Else
        Outbox.AppendText(s)
        Outbox.SelectionStart = Outbox.TextLength
        Outbox.ScrollToCaret()
        Outbox.Select()
        Outbox.Refresh()
    End If
End Sub

If I use the commented out Task.Factory code everything seems to work perfectly, except I cant control how many tasks at a time are launched, and I can't wait till all of them have finished, the for loop just launches all the tasks, and then carries on with the Output("Done....) line.

The synclock statements didn't seem to affect anything either way.

Do you have other "SyncLock Me" in the code? Might be better to "Synclock Outbox" since everything is on that object. — the_lotus, May 21 '19 at 15:37
Don't lock on `Me` or on `Outbox`, create a dedicated object to lock on. https://stackoverflow.com/questions/11775205/why-is-it-a-bad-practice-to-lock-the-object-we-are-going-to-change/11775353#11775353 — Dan Puzey, May 21 '19 at 17:28
@dbasnett Yes. A button on the form opens a file-requester and processes a CSV, storing the Datarecord objects for each line of the csv in recordList. — Peter Page, May 22 '19 at 13:19
As an aside, you should prefer `Task.Run` to `TaskFactory.StartNew` unless you have a specific reason for preferring the latter---it has some default options that are usually not correct and will produce surprising behavior. — Craig, May 22 '19 at 13:28
@PeterPage - if all of what you described is happening in the Click or metods called from the Click then the UI will be blocked. I added an example to my answer that might give you an idea. — dbasnett, May 22 '19 at 14:16

dbasnett · Accepted Answer · 2019-05-22T12:36:13.097

Give this a try

Private Sub Output(ByVal s As String)
    If Me.InvokeRequired Then
        Me.Invoke(Sub() Output(s))
        'Me.BeginInvoke(Sub() Output(s))
    Else
        Outbox.AppendText(s)
        Outbox.SelectionStart = Outbox.TextLength
        Outbox.ScrollToCaret()
        Outbox.Select()
        Outbox.Refresh()
    End If
End Sub

There may be an issue if you have events tied to Outbox, like text changed. Tested Output method with

Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
    Dim nums As New List(Of Integer)
    For x As Integer = 1 To 500
        nums.Add(x)
    Next

    'because it is in a button, run from a task
    Dim t As Task
    t = Task.Run(Sub()
                     Parallel.ForEach(nums, Sub(num)
                                                Output(num.ToString & Environment.NewLine)
                                            End Sub)
                 End Sub)
End Sub

Thanks for that, much cleaner than my original code but still not working. I'm about to update my original post with further findings. — Peter Page, May 22 '19 at 10:14
@PeterPage - How is RunParallel started? From a button perhaps? — dbasnett, May 22 '19 at 12:34

score 0 · Answer 2 · answered May 22 '19 at 13:44

If you want to go ahead with using a Task-based approach, then you certainly can control how many are launched at a time, and wait for all of them to finish. It requires some additional code for the manual management. This is discussed in some detail in Microsoft documentation: https://learn.microsoft.com/en-us/dotnet/standard/asynchronous-programming-patterns/consuming-the-task-based-asynchronous-pattern

It's not necessarily a bad thing to initiate all of the tasks immediately, then you'll be leaving it to the thread pool to take care of how many to run at a time.

If you want greater control, you can use the "throttling" design from the link. In your "pending" queue, store delegates/lambdas that will themselves kick off Task.Run. Then, as you dequeue from the "pending" queue into the "active" list, you can Invoke on the delegate/lambda to get the Task and Await Task.WhenAny on the "active" list.

One potential benefit of doing things this way is that the work in each top-level Task can be split between UI work running on the UI thread and processor-limited work running on the thread pool.

(I'm not suggesting that this is necessarily the best option for you, just trying to expand on what you should be looking at doing if you really want to pursue using Task instead of Parallel.)

Need a fix for my deadlocking issue with my parallel.foreach

2 Answers2