Add 'good vs bad' trace for shaky test -- and solve it

[abelbraaksma]

The following test may fail from time to time. It's unclear what the exact conditions are for failing it consistently:

[<Theory; ClassData(typeof<TestEmptyVariants>)>]
let ``CE empty taskSeq, GetAsyncEnumerator + MoveNextAsync multiple times`` variant = task {
    let tskSeq = getEmptyVariant variant
    use enumerator1 = tskSeq.GetAsyncEnumerator()
    do! moveNextAndCheck false enumerator1

    // getting the enumerator again
    use enumerator2 = tskSeq.GetAsyncEnumerator()
    do! moveNextAndCheck false enumerator1 // original should still work without raising
    do! moveNextAndCheck false enumerator2 // new one should also work without raising
}

It fail, among others, with input like the following (it's variant DelayDoBang):

taskSeq {
   do! longDelay ()
   do! longDelay ()
   do! longDelay ()
}

In this branch's first commit there's a TRACE_FAIL and a TRACE_SUCCESS for comparison. It appears that completed flag isn't set consistently.

[dsyme]

I think it would be worth checking the equivalent generated C# source for any uses of "volatile" or other synchronization primitives - it's likely we have a race condition somewhere

[abelbraaksma]

Apparently can also raise the following error for test
CE empty taskSeq, GetAsyncEnumerator + MoveNextAsync multiple times (variant: DelayYieldBangNested):

System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
   at FSharpy.Tests.Bug #42 -- synchronous.CE empty taskSeq\, GetAsyncEnumerator - MoveNextAsync multiple times@52.MoveNext() in D:\Projects\OpenSource\Abel\TaskSeq\src\FSharpy.TaskSeq.Test\TaskSeq.StateTransitionBug.Tests.CE.fs:line 60
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass48_0.<<InvokeTestMethodAsync>b__1>d.MoveNext() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 264
--- End of stack trace from previous location ---
   at Xunit.Sdk.ExecutionTimer.AggregateAsync(Func`1 asyncAction) in /_/src/xunit.execution/Sdk/Frameworks/ExecutionTimer.cs:line 48
   at Xunit.Sdk.ExceptionAggregator.RunAsync(Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 90

[abelbraaksma]

checking the equivalent generated C# source for any uses of "volatile" or other synchronization primitives

Thanks, that's a good suggestion. I'll have a look. These tests use mutable state with Interlocked.Increment, but the empty tests never get to mutating the state. Which is why this particular exception stood out.

[abelbraaksma]

Improved on the minimal repro. This now always fails. Also, we've now shown it is NOT a race condition:

[<Fact>]
let ``Crash Empty TaskSeq with NRE``() = task {
    let tskSeq = taskSeq {
        do! Task.Delay(50) |> Task.ofTask

        // adding any kind of yield and the problem disappears:
        //yield 10
    }

    use enumerator1 = tskSeq.GetAsyncEnumerator()
    let! (hasNext: bool) = enumerator1.MoveNextAsync()

    use enumerator1 = tskSeq.GetAsyncEnumerator()
    let! (hasNext: bool) = enumerator1.MoveNextAsync()
    let! (hasNext: bool) = enumerator1.MoveNextAsync()  // this ALWAYS throws

[abelbraaksma]

Solution: set the _machine.Data.conditional to true directly from the IValueTaskSource implementation for GetResult. Without it, the resumption incorrectly executes and jumps to places where it has invalid state, like the awaiter unset to null.

See in particular this commit: #d5d49df.

- it's likely we have a race condition somewhere

For posterity: it turned out to be a missed scenario for the special case of awaited tasks in empty sequences. There was no race condition.