Seemingly random NullReferenceException in async state machine

[Skyppid]

Seemingly random NullReferenceException in async state machine

[sakno]

Release 5.3.1 has been published

[Skyppid]

I could nail it down to the following. This snippet shows a part of the generated transition lambda:

    .Block() {
        .Switch ($var2.StateId) {
        .Case (1U):
                .Goto state_1 { }
        .Case (2U):
                .Goto state_2 { }
        .Default:
                .Default(System.Void)
        };
        // Other code
        .Block() {
            (($var2.State).Rest).Item1 = .Call (.Call (($var2.State).Item7).MapEvent(
                ($var2.State).Item1,
                (ChangeHistoryService.IterationModel.Changes.EventChangeDtoBase`1[ChangeHistoryService.IterationModel.Changes.Projects.FileCreatedChange])($var2.State).Item7,
                ($var2.State).Item2,
                ($var2.State).Item3)).GetAwaiter();
            .If (
                .Call $var2.MoveNext(
                    (($var2.State).Rest).Item1,
                    1U)
            ) {
                .Default(System.Void)
            } .Else {
                .Return end_async_method { }
            };
            .Label
            .LabelTarget state_1:;
            .Call ((($var2.State).Rest).Item1).GetResult()

The call fails when trying to execute the last line. In this case it seems like the state is not defined. I'm not really getting much insight as to what is causing this:

Any idea?

PS: When I call the compiled method with .Result no exception is thrown. There must be something off with the state. I have not much insight into the workings and it's hard to guess. I ran through the entire code down into the CLR. I can't find any reason why the State becomes missing. During the creation of the state machine box it seems to be there.

Help would really be appreciated here. Thanks!

[sakno]

Do you have repro code?

[sakno]

Some way to generate PDBs for it so I can step into the methods?

Unfortunately, no. This is supported in .NET Framework only.

[Skyppid]

@sakno I added you to a small reproduction repo in our organization. I guess you should be notified with a link, if not tell me please.

When you run it you can see that the resulting DTO has all the "steps" but each step has only one change although it should have multiple. If you set a breakpoint in the catch handler of the PolymorphicMapper.MapChanges method, then you'll see the exception thrown.

Hope you can figure things out with this :) Thanks for the help!

Tip: Using https://github.com/MapsterMapper/ExpressionDebugger I managed to at least get a bit more information out of it. Using Rider I could step into most parts (except the generated method unfortunately). Helps a lot. It then also actually hit an exception breakpoint itself when the mapping was done where you could do a single step and land inside the AsyncStateMachine.Next method where you could inspect the state of the machine (and the empty State).

[sakno]

I see the link, thanks. The problem is that I need an isolated repro code to convert it into the test.

[Skyppid]

Ah okay. Well, it's hard to say what is required to reproduce it. Not sure what can be stripped and how a test for that should look like... I'll try to strip everything around it as much as possible.

[Skyppid]

Alright, I removed everything that is not needed. Still reproduces. Should be few enough to convert it into a test, I hope. Can you work with that now?

[sakno]

Reproduced with the following test:

[Fact]
public static async Task RegressionIssue234()
{
    var method1 = typeof(LambdaTests).GetMethod(nameof(DoNothingAsync), BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.DeclaredOnly);
    var method2 = typeof(LambdaTests).GetMethod(nameof(DoYieldAsync), BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.DeclaredOnly);
    var lambda = AsyncLambda<Func<Task<int>>>((ctx, result) =>
    {
        Await(Expression.Call(method2));
        Await(Expression.Call(method1));
        Assign(result, 42.Const());
    }).Compile();

    for (var i = 0; i < 100; i++)
    {
        Equal(42, await lambda.Invoke());
    }
}

private static Task DoNothingAsync() => Task.CompletedTask;

private static async Task DoYieldAsync()
{
    await Task.Yield();
}

[Skyppid]

Wonderful, good to hear it helped you nail it down to a small test :) I'm really curious as to what is causing this issue. I couldn't identify any issue in the control flow yet.

Btw: You should maybe consider adding some kind of donation option to your repository (if just for a coffee) ;)

[sakno]

It's fixed. Could you check it from develop branch? The main problem was cleanup inside of IAsyncStateMachine.MoveNext implementation for pooling versions of state machine. When pooling is used, the fields should not be set after SetResult to avoid concurrency with the .NET pool of value tasks. The root case is the following:

1. Async method reaches final state with StateId = 0
2. It calls SetResult that resumes the caller
3. Caller consumes ValueTask using GetResult. This method returns boxed state machine back.
4. A new execution takes this cached state machine
5. In the same time, the execution flow from step 2 continues and erases the fields of the boxed state machine which is taken by another async flow.

[sakno]

Moreover, when PoolingAsyncValueTaskMethodBuilder returning back the boxed state machine, it sets all fields to default. No need to do that twice.

[Skyppid]

Looks splendid, works like a charm. I really expected it to be more, but it makes perfect sense that a pooled state machine might be reused before it should be.

Thank you very much!