none
Bug in AsyncEnumerable or AsyncCTP?

    Question

  • I think I've found a bug in either the AsyncCTP or the IAsyncEnumerable implementation.

    I have an observable source of integers, generated with time, converted to an async enumerable. When I use this IAsyncEnumerable in a while loop (i.e. while (await ...)), the resulting continuation only appears to run once and then hangs (or never comes back).

    If I remove the time delay (see [1] below), the loop runs to completion. If I add a side effect (see [2] below) after the call to ToAsyncEnumerable, the loop runs to completion.

    I have written a minimal test case that exhibits the bug. In the continuation of the while(await ...) loop, we're pushing each integer on the stack and sleeping (blocking) for a bit. At the end of the test, we verify that all integers have been pushed in a "reasonable" amount of time. Here is the code:

    using System;
    using System.Collections.Generic;
    using System.Concurrency;
    using System.Diagnostics;
    using System.Linq;
    using System.Threading;
    using System.Threading.Tasks;
    using Microsoft.VisualStudio.TestTools.UnitTesting;
    
    namespace AsyncAwaitBugTest
    {
      [TestClass]
      public class AsyncAwaitBugTests
      {
        [TestMethod]
        public void TestPushPopAsync()
        {
          var stack = new Stack<int>();
          var count = 10;
    
          var observable = Observable.GenerateWithTime(
            0,
            i => i < count,
            i => i + 1,
            i => i,
            i => TimeSpan.FromMilliseconds(1), // change this to 0 to avoid the problem [1]
            Scheduler.ThreadPool);
    
          var task = DoSomethingAsync(observable, stack);
    
          // we give it a timeout so the test can fail instead of hang
          task.Wait(TimeSpan.FromSeconds(2));
    
          Assert.AreEqual(10, stack.Count);
        }
    
        private async Task DoSomethingAsync(IObservable<int> observable, Stack<int> stack)
        {
          var ae = observable
            .ToAsyncEnumerable()
            //.Do(i => Debug.WriteLine("Bug-fixing side effect: " + i))   // [2]
            .GetEnumerator();
    
          while (await ae.MoveNext())
          {
            var i = ae.Current;
            Debug.WriteLine("Doing something with " + i);
            Thread.Sleep(50);
            stack.Push(i);
          }
        }
      }
    }
    

    All libraries are up-to-date as of the time of this writing:

    Visual Studio 2010 Premium: version 10.0.30319.1

    .NET 4.0 runtime version v4.0.30319

    AsyncCtpLibrary.dll version 1.0.3951.36518

    System.CoreEx.dll, System.Linq.Async.dll, System.Reactive.dll version 1.0.2838.104

     

    Any advice or suggestions would be greatly appreciated. Thanks!

    • Edited by Aaron Olson Wednesday, February 09, 2011 7:38 PM formatting issues
    Wednesday, February 09, 2011 7:36 PM

Answers

All replies

  • The "smallest" side-effect I've found that avoids the bug is Thread.Sleep(0) instead of writing a string somewhere.

    Presumably, all that's needed is to surrender the thread.

    Monday, February 14, 2011 3:51 PM
  • I also ran into this problem, and have traced it to a bug in the implementation of ToAsyncEnumerable<TSource>(). In this method, and in the private ToAsyncEnumerableObserver<TSource> class, TaskCompletionSource.SetResult() is called inside a lock. Despite its innocent-sounding name, SetResult() has a side effect of running any continuations waiting for the task to complete. In general, it's a bad idea to run arbitrary callback code inside of a lock. In your example the continuation results in a call to MoveNext() which re-acquires the lock (since it's on the same thread which already has the lock), violating some of the locking logic used to decide whether to dequeue or defer for the next value. Introducing the .Do() after ToAsyncEnumerable() avoids the bug because the continuation now no longer calls MoveNext(). In fact .Do(i => {}) is sufficient to avoid the bug.

    The fix is fairly simple - move the call to SetResult() and SetException() outside of the lock region. Hopefully the team can get this on the list for the next release.

    Steve.

    Monday, February 14, 2011 6:29 PM
  • Thanks for reporting this.  It will be fixed soon.
    • Marked as answer by fixedpoint Wednesday, February 23, 2011 1:11 AM
    Wednesday, February 23, 2011 1:11 AM