Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

"Malformed contract section" when rewriting async methods #112

Closed
roji opened this issue Jul 10, 2015 · 14 comments
Closed

"Malformed contract section" when rewriting async methods #112

roji opened this issue Jul 10, 2015 · 14 comments

Comments

@roji
Copy link
Member

roji commented Jul 10, 2015

Encountered while building Npgsql:

Severity    Code    Description Project File    Line
Error   CC1017  Malformed contract section in method 'Npgsql.NpgsqlBuffer+<SkipAsync>d__78.MoveNext'    Npgsql  C:\projects\npgsql\src\Npgsql\NpgsqlBuffer.cs   600
Error   CC1017  Malformed contract section in method 'Npgsql.NpgsqlDataReader+<GetFieldValueInternalAsync>d__133`1.MoveNext'    Npgsql  C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs   1861
Error   CC1017  Malformed contract section in method 'Npgsql.NpgsqlLargeObjectStream+<SeekAsync>d__34.MoveNext' Npgsql  C:\projects\npgsql\src\Npgsql\NpgsqlLargeObjectStream.cs    300
Error   CC1017  Malformed contract section in method 'Npgsql.NpgsqlLargeObjectStream+<SetLengthAsync>d__36.MoveNext'    Npgsql  C:\projects\npgsql\src\Npgsql\NpgsqlLargeObjectStream.cs    300
Error       The command ""C:\Program Files (x86)\Microsoft\Contracts\Bin\ccrewrite.exe" "@Npgsqlccrewrite.rsp"" exited with code 4. Npgsql      

Note that the files and line numbers are bogus: these async methods are in GeneratedAsync.cs (there's some code generation going on). In any case, the first methods is simply this:

        internal async Task SkipAsync(long len)
        {
            Contract.Requires(len >= 0);
            if (len > ReadBytesLeft)
            {
                len -= ReadBytesLeft;
                while (len > Size)
                {
                    Clear();
                    await EnsureAsync(Size);
                    len -= Size;
                }

                Clear();
                await EnsureAsync((int)len);
            }

            ReadPosition += (int)len;
        }

Am not sure what is causing this: a simple async method with Contract.Requires seems OK...

@SergeyTeplyakov
Copy link
Contributor

I can reproduce this issue. Will look at it more closely tomorrow.

@roji
Copy link
Member Author

roji commented Jul 10, 2015

OK great!

@roji
Copy link
Member Author

roji commented Jul 10, 2015

I can confirm that after commenting out contracts in the async methods above, Npgsql builds fine so this seems to be the last issue.

@SergeyTeplyakov
Copy link
Contributor

Thanks a lot. Hope to push both fixes tomorrow.

@roji
Copy link
Member Author

roji commented Jul 10, 2015

Thanks for the very quick help, looking forward to using CC in VS2015.

@SergeyTeplyakov
Copy link
Contributor

Ok, minimum repo is:

internal async Task WillFail(string str)
{
    Contract.Requires(str != null);
    await Task.Delay(42);
    await Task.Delay(42);
}

I.e. Requires and more than 1 await.

@SergeyTeplyakov
Copy link
Contributor

Ok, small update.

The reason with this bug that Rosly compiler will generate following code:

void IAsyncStateMachine.MoveNext()
            {
                int num = this.<>1__state;
                try
                {
                    TaskAwaiter taskAwaiter;
                    TaskAwaiter taskAwaiter2;
                    if (num != 0)
                    {
                        if (num == 1)
                        {
                            taskAwaiter = this.<>u__1;
                            this.<>u__1 = default(TaskAwaiter);
                            this.<>1__state = -1;
                            goto IL_E8;
                        }
                        Contract.Requires(this.str != null);
                        taskAwaiter2 = Task.Delay(42).GetAwaiter();
                        if (!taskAwaiter2.IsCompleted)
                        {
                            this.<>1__state = 0;
                            this.<>u__1 = taskAwaiter2;
                            Foo.<Method>d__0 <Method>d__ = this;
                            this.<>t__builder.AwaitUnsafeOnCompleted<TaskAwaiter, Foo.<Method>d__0>(ref taskAwaiter2, ref <Method>d__);
                            return;
                        }
                    }
                    else
                    {
                        taskAwaiter2 = this.<>u__1;
                        this.<>u__1 = default(TaskAwaiter);
                        this.<>1__state = -1;
                    }
                    taskAwaiter2.GetResult();
                    taskAwaiter2 = default(TaskAwaiter);
                    taskAwaiter = Task.Delay(42).GetAwaiter();
                    if (!taskAwaiter.IsCompleted)
                    {
                        this.<>1__state = 1;
                        this.<>u__1 = taskAwaiter;
                        Foo.<Method>d__0 <Method>d__ = this;
                        this.<>t__builder.AwaitUnsafeOnCompleted<TaskAwaiter, Foo.<Method>d__0>(ref taskAwaiter, ref <Method>d__);
                        return;
                    }
                    IL_E8:
                    taskAwaiter.GetResult();
                    taskAwaiter = default(TaskAwaiter);
                }
                catch (Exception exception)
                {
                    this.<>1__state = -2;
                    this.<>t__builder.SetException(exception);
                    return;
                }
                this.<>1__state = -2;
                this.<>t__builder.SetResult();
            }

I.e. Contract.Requires is not in the beginning of the state machine initialization. Not sure is it ok or not.

@SergeyTeplyakov
Copy link
Contributor

@roji As a super nasty workaround could you try to add await Task.Yield(); at your method and run the tests?

It seems that the only case when ccrewrite will fail, is when you have exactly two awaits in your code!

P.S. I'm working on the solution. This is just an experiment!

@roji
Copy link
Member Author

roji commented Jul 10, 2015

Am unfortunately out... I'll try it out tomorrow morning if you haven't
done something more.... Permanent :)

Am Freitag, 10. Juli 2015 schrieb Sergey Teplyakov :

@roji https://github.com/roji As a super nasty workaround could you try
to add await Task.Yield(); at your method and run the tests?

It seems that the only case when ccrewrite will fail, is when you have
exactly two awaits in your code!

P.S. I'm working on the solution. This is just an experiment!


Reply to this email directly or view it on GitHub
#112 (comment)
.

@SergeyTeplyakov
Copy link
Contributor

@roji Sounds good!

@SergeyTeplyakov
Copy link
Contributor

Fix is ready. Stay tuned!

SergeyTeplyakov added a commit to SergeyTeplyakov/CodeContracts that referenced this issue Jul 11, 2015
…ments.

This fix resolves the issue microsoft#112. Now ccrewrite will recognize new
pattern that roslyn-based compiler will use for async methods with 2
await statements.
@SergeyTeplyakov
Copy link
Contributor

Fixed by #114.

@roji
Copy link
Member Author

roji commented Jul 11, 2015

Just to say that the new release of CC fixes the issues I ran into with Npgsql, thanks for the speedy work!

@SergeyTeplyakov
Copy link
Contributor

Feel free to post any issues you have with vs2015, or even with older version!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants