.Net 4.5 killed my TPL, now what?

Question

Exhibit 1: some code wrapping an Async (not async!) network call into a Task

public static Task<byte[]> GetAsync(IConnection connection, uint id)
{
    ReadDataJob jobRDO = new ReadDataJob();

    //No overload of FromAsync takes 4 extra parameters, so we have to wrap
    // Begin in a Func so that it looks like it takes no parameters except 
    // callback and state
    Func<AsyncCallback, object, IAsyncResult> wrapped = (callback, state) =>
                jobRDO.Begin(connection, 0, 0, id, callback, state);

    return Task<byte[]>.Factory.FromAsync(wrapped, ar =>
    {
        ErrorCode errorCode;
        UInt32 sError;
        UInt32 attribute;
        byte[] data = new byte[10];
        jobRDO.End(out errorCode, out sError, out attribute, out data);
        if(error != ErrorCode.NO_ERROR)  throw new Exception(error.ToString());
        return data;
    }, jobRDO);
}

Installing .Net 4.5 (not pointing VS at it, nor recompiling) stops this working. The callback is never invoked.

Any ideas what could be causing this, and otherwise, what I can do to try and further narrow down the root cause of the problem or work around it?

Answer 1

Re-edit: I've exchanged several emails with Stephen Toub. Below I attempt to merge my original answer and his answers into a coherent whole.

tl;dr to work around this, force CompleteSynchronously to always return false (caveat non-lector).

---

MSDN documentation of .Net 4.5 'breaking' changes

Immediately after posting this question, I clicked through to a related question, and ended up at "Application Compatibility in the .NET Framework 4.5", which has this to say about FromAsync:

Change: The IAsyncResult implementation must complete synchronously and its CompletedSynchronously property must return true for the resulting task to complete.

Impact: The resulting task will not complete if an IAsyncResult implementation does not complete synchronous execution but its CompletedSynchronously property returns True.

Ironically, (or infuriatingly), the page for CompletedSynchronously states:

Notes to Implementers: Most implementers of the IAsyncResult interface will not use this property and should return false.

---

Stephen Toub clarified this with the following:

The table at http://msdn.microsoft.com/en-us/library/hh367887%28v=VS.110%29.aspx#core, and specifically the description of the “Change”, is wrong (...).

There was a change in .NET 4.5 to FromAsync, but it wasn’t that all IAsyncResult.CompletedSynchronously implementations must return true: that wouldn’t make any sense. The change was that FromAsync actually looks at the IAsyncResult’s CompletedSynchronously now (it didn’t look at it at all in .NET 4), and thus it expects it to be accurate. As such, if you had a buggy IAsyncResult implementation, FromAsync might still have worked in .NET 4, whereas with .NET 4.5, it’s less likely to work with a buggy implementation.

Specifically, it’s ok if IAsyncResult.CompletedSynchronously returns false. However, if it returns true, the IAsyncResult must have in fact completed synchronously. If CompletedSynchronously returns true but the IAsyncResult has not completed, you have a bug that needs to be fixed, and it’s likely that the Task returned from FromAsync will not complete correctly.

The change was made for performance reasons.

---

Returning to my problem code

Here is his very helpful analysis, which I include in full as it will probably be useful for other implementers of IAsyncResult:

The problem appears to be that the library you’re using has a very faulty implementation of IAsyncResult; in particular, it’s implementing CompletedSynchronously incorrectly. Here’s their implementation:

public bool CompletedSynchronously
{
    get { return _isCompleted; }
}
public bool IsCompleted
{
    get { return _isCompleted; }
}

Their _isCompleted field indicates whether the asynchronous operation has completed, which is fine, and it’s fine to return this from IsCompleted, since that property is meant to indicate whether the operation completed or not. But CompletedSynchronously can’t just return that same field: CompletedSynchronously needs to return whether the operation completed synchronously, i.e. whether it completed during the call to BeginXx, and it must always return the same value for a given IAsyncResult instance.

Consider the standard pattern for how IAsyncResult.CompletedSynchronously is used. Its purpose is to allow the caller of BeginXx to continue doing the follow-on work, rather than having the callback due the work. This is particularly important for avoiding stack dives (imagine a long sequence of asynchronous operations that all actually completed synchronously: if the callbacks handled all of the work, then each callback would initiate the next operation, whose callback would initiate the next operation, but because they were completing synchronously, their callbacks would also be invoked synchronously as part of the BeginXx methods, so each call would get deeper and deeper on the stack, until it potentially overflowed):

IAsyncResult ar = BeginXx(…, delegate(IAsyncResult iar) =>
{
    if (iar.CompletedSynchronously) return;
    … // do the completion work, like calling EndXx and using its result
}, …);
if (ar.CompletedSynchronously)
{
    … // do the completion work, like calling EndXx and using its result
}

Note that both the caller and the callback use the same CompletedSynchronously property to determine which of them runs the callback. As such, CompletedSynchronously must always return the same value for this particular instance. If it doesn’t, erroneous behavior can easily result. For example, their implementation has CompletedSynchronously returning the equivalent of IsCompleted. So imagine the following sequence of events:

• BeginXx is called and initiates the async operation
• BeginXx returns to its caller, which checks CompletedSynchronously, which is false, because the operation hasn’t completed yet.
• Now the operation completes and the callback is invoked. The callback sees that CompletedSynchronously is true, and so doesn’t do any of the subsequent work, because it assumes the caller did it.
• And now no one’s run or will run the callback.

In short, the library has a bug. If you changed CompletedSynchronously to return true, you masked this problem, but you likely caused another: if the caller (in your case, FromAsync) thinks that the operation has already completed, it’ll proceed to immediately call the EndXx method, which will block until the async operation has completed, so you’ve turned your asynchronous operations into synchronous ones. Have you tried just always returning false from CompletedSynchronously instead of always returning true?