Async CTP Refresh - compiler bug fixes

Article
04/16/2011

Async CTP Refresh - Compiler Bug fixes

The big news about the Async CTP Refresh is that it enabled development on SP1 and for Windows Phone 7, and came with a new EULA. But there were also a few bug fixes...

We heard about some of these bugs from community feedback. Thank you. Such feedback is enormously useful to us.

This blog post lists the only compiler bugs fixed in the Async CTP Refresh. (In addition there were several library bugs fixed in the dataflow libraries). I want to spell out in detail some of these bugs we've found and fixed. This will give you an idea of how deeply we looked, and the kinds of bugs that arose. I also want to publically thank the users who alerted us to two of these bugs.

Fixed bug: a C# race condition in "finally" blocks

This bug was found by our QA department, and also by a user who emailed us directly, and also by the author of this blog: https://fredrikeriksson.me/post/3276445803/a-subtle-race-condition-in-the-asyncctp

// Bug: in the first CTP, if you had high concurrency, then sometimes the finally
// block would not be executed, and sometimes it would be executed more than once
try { await e; }
finally { Console.WriteLine("f"); }

// The first CTP generated this:

class StateMachine{ bool $doFinallyBodies;

void MoveNext() { $doFinallyBodies = true; try { var $temp = e.GetAwaiter(); $doFinallyBodies = false; if (temp.BeginAwait(cont)) return; cont: $doFinallyBodies = true; $temp.EndAwait(); } finally { if ($doFinallyBodies) { Console.WriteLine("f"); } } }}

// The CTP Refresh generates this:

class StateMachine{ void MoveNext() { bool $doFinallyBodies = true ... ... ... }}

The field "$doFinallyBodies" is used so that, if we had to return early due to an "await" statement, then we'd bypass the finally bodies that the user had written. They should only be executed on normal exit.

The bug was that by accident I'd made $doFinallyBodiesa field. It should have been a local. As a field, if there was high concurrency and lots of threads running the same async method, then one thread might overwrite the shared $doFinallyBodies at the wrong time.

VB did not have this bug, since in VB it was already a local.

Fixed bug: another C#/VB race condition in "finally" blocks

This bug was found by user "Soonts" on StackOverflow, https://stackoverflow.com/questions/5032784/async-ctp-and-finally.

' Bug: in the first CTP, this would print "1" then "finally".
' But it's supposed to print "finally" then "1".
Sub Main()
Console.WriteLine(f().Result)
End Sub

Async Function f() As Task(Of Integer)
    Try
        Await TaskEx.Yield()
        Return 1
    Finally
        Console.WriteLine("finally")
    End Try
End Function

' The first CTP generated this:Class StateMachine Dim $builder As AsyncTaskMethodBuilder(Of Integer) Sub MoveNext() Try Await TaskEx.Yield() $builder.SetResult(1) Catch ex As Exception $builder.SetException(ex) End Try End SubEnd Class

' The CTP Refresh generates this:Class StateMachine Dim $builder As AsyncTaskMethodBuilder(Of Integer) Sub MoveNext() Dim $ReturnVariable As Integer Try Await TaskEx.Yield() $ReturnVariable = 1 GoTo exit Catch ex As Exception $builder.SetException(ex) Return End Try exit: $builder.SetResult(ReturnVariable) End SubEnd Class

Fixed bug: a VB compiler bug with caught exception variables

Use of the exception variable would often have failed PEVerify, and in code like below where you're using the exception variable it would have caused a runtime exception. The root cause was that VB compiler generates IL with a temporary local variable for "ex". In an async method, the temporary local variable had to be lifted into the state machine.

' Bug: in the first CTP, runtime crashes in some uses of the caught exception variable.
' This has been fixed in the CTP Refresh
Async Sub f()
    Try
        Throw New Exception("test")
    Catch ex As Exception
        Dim lambda = Sub()
                         Console.WriteLine(ex) ' crash right here
                     End Sub
        lambda()
    End Try
End Sub

Fixed bug: a VB compiler bug with Select statements in asyncs/iterators

Select Case statements in VB async+iterator methods had a chance of incorrect codegen, depending on the exact types used by the statement and the ranges of the Case clauses. It sometimes failed PEVerify, and more often just picked the wrong Case branch. Once again the problem was one of IL with temporary local variables, this time for the Select Case operand. This bug was hard to track down because VB does aggressive optimization on Select Case statements, and only certain optimizations would expose the bug.

' Bug: in the first CTP, if invoked with "10", this would print ERROR
' This has been fixed in the CTP refresh
Async Sub f(y As Long)
    Select Case y
        Case 8 To 10
            Console.WriteLine("SUCCESS")
        Case Else
            Console.WriteLine("ERROR")
    End Select
End Sub

Fixed bug: a VB compiler bug with late-bound For loops in async/iterators

Late-bound VB For loops in VB async+iterator methods would often have failed PEVerify and crashed at runtime. Once again the problem was one of IL with temporary local variables, this time for the upper bound of the For loop, and the fix was to lift it to be a field in the state machine. You might get the feeling that a lot of our bugs were due to the need to lift temporary local variables. Once we twigged onto this, we systematically tracked down every last case that needed it.

' Bug: in the first CTP, late-bound For loop crashed
' This has been fixed in the CTP Refresh
Async Sub f()
    Dim x As Object = 4
    For i = 1 To x
        Console.WriteLine(i)
    Next
End Sub

Note: on the Async Forums, user MattMc3 pointed out a For loop bug that still exists: if the For loop has a Step that isn't constant, then it won't work. The reason is that (sigh) the CTP fails to lift the "step variable temporary".

Fixed bug: a VB compiler bug with type constraints in asyncs/iterators

Generic constraints in async/iterator methods would have caused problems if they referred to other generic parameters of the async method. This usually manifest as a VS crash; sometimes it failed PEVerify instead. The bug was because, when we lift the method into a state machine class, we have to turn the method's generic parameters into generic parameters of the state machine class, and we have to remember to rewrite the constraints accordingly.

' Bug: in the first CTP, this could would sometimes crash the IDE.
' This has been fixed in the CTP Refresh
Async Sub CreateListElement(Of TElem, TList As {New, List(Of TElem)})()
Dim l2 As New TList()
End Sub

Fixed bug: a pre-existing C# bug with nullables

In C#, a method with nullable equality tests would sometimes evaluate the operands more than once. We seemed to hit this more in our async testing, but we discovered that it was caused by a pre-existing bug in VS2010, one that arose from optimizations in the handling of nullables. This has been fixed in the CTP Refresh.

// Bug: in the first CTP, this would do incorrect codegen
async void g() {
Task<bool> t = null;
bool b = new Nullable<bool>(true) == await t;
}

// Bug: in VS2010, this would print "f" twice.
// This has been fixed in the CTP Refresh
void Test() {
bool b = (new Nullable<bool>(true)) == f();
Console.WriteLine(b);
}

bool f() {
Console.WriteLine("f"); return true;
}

Fixed bug: a pre-existing C# bug using "base.property"

In C#, an async method which accessed "base.property" would cause incorrect codegen in the case where that property had different accessibility of its getter and setter. We discovered that this was caused by a pre-existing bug in VS2010, to do with the similar case of iterators. It would produce incorrect codegen, fail PEVerify, and sometimes throw an exception at runtime. The root cause of the bug is that both iterator and async methods must be lifted into a class. This class can no longer call any protected "base" method since it lacks accessibility. Instead it needs to generate a stub method which does have that accessibility. And the logic for deciding when that stub method was needed, failed to take into account the potential different visibility of setters and getters.

// Bug: in the first CTP, both "f" and "g" would produce bad codegen
// that failed PEVerify. This has been fixed in the CTP Refresh.
class C : B
{
IEnumerator f() { base.p = 1; yield break; }
async void g() { base.p = 1; }
}

class B
{
public virtual int p { private get; set; }
}

Fixed bug: VB would sometimes crash VS if you double-dotted

In the first CTP, in VB, if you had a variable whose type was inferred from an expression involving Await, and if you then double-dotted off this variable, it would crash VS.

Dim s = Await (New WebClient).DownloadStringTaskAsync(https://a.com)
Dim len = s.ToUpper().Length ' would crash VS in the first CTP

The reasons are pretty bizarre. When you dot off something, the intellisense engine needs to know the type. It finds the type by "keyhole analysis", i.e. only looking at the lines in question, without regard to the containing method. Indeed it didn't know that the containing method was async. There was a mismatch between intellisense (who therefore thought the Await was illegal) and the compiler (who knew it was legal). This mismatch caused the crash. The fix is for intellisense not to judge legality or otherwise of an Await.

Comments

Anonymous
April 18, 2011
now that's transparency ! thank you !
Anonymous
May 04, 2011
The pre-existing C# bug with nullables exists in .NET 3.5 as well. Replacing "new Nullable<bool>(true)" with "true" avoids the problem.

Partager via