0

I'm working on an application where at some point i'm doing some CPU intensive calculation on the kinematics of an object. At some point i'm using Task.Run() to call the function doing the work, this leads to some unexpected results, i'm not sure if this would be considered a race condition or if it has some other name. My real code is rather expansive, so i have reduced the issue to what i believe to be a minimal working example, to be run in a .net framework console application.

For the MWE consider the following class, it has 3 fields and a constructor initializing them. It also has a report() sub for easier debugging.

Public Class DebugClass
    Public Variable1 As Double
    Public Variable2 As Double
    Public Variable3 As Double

    Public Sub New(Variable1 As Double, Variable2 As Double, Variable3 As Double)
        Me.Variable1 = Variable1
        Me.Variable2 = Variable2
        Me.Variable3 = Variable3
    End Sub

    Public Sub Report()
        Console.WriteLine()
        Console.WriteLine("Variable1: {0},Variable2: {1},Variable3: {2}", Variable1, Variable2, Variable3)
    End Sub
End Class

I also have another helper function which replaces the CPU intensive work that my real application would have, with a random delay between 0 and 1 second :

Public Async Function RandomDelayAsync() As Task
    Await Task.Delay(TimeSpan.FromSeconds(Rnd()))
End Function

For demonstration purposes I have 2 version of my "work" function; an Async an non-Async version. Each of these functions takes and instance of DebugClass as a parameter, pretends to do some work on it and then simply returns the same object that it got as an input. :

'Async version
Public Async Function DoSomeWorkAsync(WorkObject As DebugClass) As Task(Of DebugClass)
    Await RandomDelayAsync()
    Return WorkObject
End Function

'Synchronous version
Public Function DoSomeWork(WorkObject As DebugClass) As DebugClass
    RandomDelayAsync.Wait()
    Return WorkObject
End Function

Lastly, i have a WaiterLoop, this function Awaits the created tasks for one to complete, prints the returned object's fields to the console and removes it from the list of tasks. It then waits for the next one to complete. In my real application i would do some more calculations here after i get the results from the individuals tasks, to see which parameters give the best results.

Public Async Function WaiterLoop(TaskList As List(Of Task(Of DebugClass))) As Task
    Dim Completed As Task(Of DebugClass)
    Do Until TaskList.Count = 0
        Completed = Await Task.WhenAny(TaskList)
        Completed.Result.Report()
        TaskList.Remove(Completed)
    Loop
End Function

Now, first consider this version of my Main() function:

Sub Main()
    Randomize()

    Dim Tasklist As New List(Of Task(Of DebugClass))

    Dim anInstance As DebugClass
    For var1 As Double = 0 To 5 Step 0.5
        For var2 As Double = 1 To 10 Step 1
            For Var3 As Double = -5 To 0 Step 1
                anInstance = New DebugClass(var1, var2, Var3)
                'adding an Async task to the tasklist
                Tasklist.Add(DoSomeWorkAsync(anInstance))
            Next
        Next
    Next

    WaiterLoop(Tasklist).Wait()
    Console.ReadLine()

End Sub

The output here is exactly as I would expect, the tasks all complete and for each of the parameter combinations made a line is printed to the console. All's good so far, the problem i'm facing arrises when this line:

Tasklist.Add(DoSomeWorkAsync(anInstance))

Is replaced with this line

Tasklist.Add(Task.Run(Function() DoSomeWork(anInstance)))

In this new version i don't call the Async version of the work function, instead i'm using Task.Run To run a normally synchronous function on a worker thread. This is where the s**t hits the fan. Suddenly, the output is not as expected anymore;

'This is the type of output i now get:
Variable1: 1.5,Variable2: 7,Variable3: -1

Variable1: 5,Variable2: 10,Variable3: 0

Variable1: 5,Variable2: 10,Variable3: 0

Variable1: 5,Variable2: 10,Variable3: 0

Variable1: 5,Variable2: 10,Variable3: 0

Variable1: 5,Variable2: 10,Variable3: 0

Somehow all of the tasks i created now seem to be referring to the same instance of DebugClass, as everytime a tasks completes, the same output is printed. I don't understand why this happens, because i'm creating a new instance of DebugClass before each time i start a new task: anInstance = New DebugClass(var1, var2, Var3) followed by Tasklist.Add(Task.Run(Function() DoSomeWork(anInstance))). As soon as i assign a new instance of DebugClass to AnInstance, it "forget's" the previous instance it was storing, right?. And the instance referenced by each of the created tasks ought to be independent of the ones referred to by the other tasks?

Clearly, I am mistaken, but i would appreciate it if someone could explain to me what's going on here.

I also wonder why one is faster than the other, but i will save that for a separate question unless it's related to this issue.

Thank you for taking the time to read this.

1 Answers1

3

Lambdas (ie Function() DoSomeWork(anInstance)) 'close'* on a reference to a variable NOT on its current value.

Thus Function() DoSomeWork(anInstance) means 'when you come to run perform the DoSomeWork method on the current value of anInstance'.

You only have one instance of anInstance because you declared it outside the loop.

Quick fix: Move the Dim anInstance As DebugClass statement inside the inner loop, this gives you one variable instance per loop, which is what you want.

See also Captured variable in a loop in C#, which is this basically the same question in c# and has some useful discussion/links in the comments

*Closures are a big topic, I'd suggest reading https://en.wikipedia.org/wiki/Closure_(computer_programming). Happy to discuss further in comments.

tolanj
  • 3,651
  • 16
  • 30
  • This was useful, thank you. I had seen mention of the necessity to make a local copy of a variable before passing it to a lambda, but i guess it never quite sunk in. Especially the second sentence in you answer makes it very clear. – AConfusedSimpleton Nov 08 '19 at 11:47