Async Create hanging while publishing observable

Question

Consider the following code:

var xs = Observable.Create<Unit>(async o => 
{
    await Task.Delay(10);
    o.OnError(new Exception());
}).Replay().RefCount();

xs.Subscribe(x => Console.WriteLine(x));
xs.Subscribe(x => Console.WriteLine(x), ex => Console.WriteLine(ex.Message));
await xs.DefaultIfEmpty();

The sequence above doesn't throw any exceptions and never completes.

I made the following observations:

1. Removing the first subscription enables error propagation - exception is thrown in Subscribe context (last line)
2. Removing .Replay().RefCount() enables error propagation - exception is thrown in Subscribe context (last line)
3. Removing await Task.Delay(10) enables error propagation - exception is thrown in OnError call (within Create method). Surprisingly, switching two Subscribe methods makes exception thrown at Subscribe context (last line).

That being said, I am asking whether the following issues are by design:

1. Observable sequence in the above scenario never being completed
2. The fact that exception is sometimes thrown inside Create method, and other times - at Subscribe context.

If this is by design, what would you recommend as a workaround? How do I publish my sequences so that all of my clients (observers) can safely handle exceptions in this case? Current behavior seems so arbitrary, especially for library creators. It also makes debugging very painful. Please advise.

Answer 1

First to answer your questions:

1. No it's not by design. It is a bug.
2. "Workaround" is to not mix TPL and Reactive. You can hit funny things like this.

The following works as expected:

var xs = Observable.Throw<Unit>(new Exception())
    .Delay(TimeSpan.FromMilliseconds(10))
    .Replay()
    .RefCount();

This causes exceptions to be raised on the first .Subscribe and the await xs.DefaultIfEmpty() call. You get two exceptions because of the delay: Multiple threads are running.

---

As for why this is happening, here's a start:

That first Subscribe code basically translates to the follow. (See source):

xs.Subscribe(x => Console.WriteLine(x), Stubs.Throw, Stubs.Nop);

public static class Stubs
{

    public static readonly Action Nop = delegate
    {
    };

    public static readonly Action<Exception> Throw = delegate (Exception ex)
    {
        var edi = ExceptionDispatchInfo.Capture(ex);
        edi.Throw();
    };
}

If you breakpoint into the Stubs class, you'll see that it gets in there and tries to throw the exception. However, the exception doesn't bubble up, most likely due to some weird TPL/ReplaySubject interaction.

Answer 2

I think this may have to do with the dynamics of pipeline teardown.

This is our expected behavior for Replay (which uses ReplaySubject internally).

However, whether the error notification propagates before the pipeline tears down seems to be a matter of of timing. When using a ReplaySubject directly, it works as expected, even with OnError in Subscribe.

        var xs = new ReplaySubject<Unit>();
        var sxs = Observable.Create<Unit>(async o =>
        {
            await Task.Delay(10);
            o.OnError(new Exception("ERR"));

        }).Subscribe(xs);


        xs.Subscribe(x => Console.WriteLine(x), ex => Console.WriteLine(ex.Message));
        xs.Subscribe(x => Console.WriteLine(x), ex => Console.WriteLine(ex.Message));

        xs.DefaultIfEmpty().Wait();

        Console.WriteLine("-end-");
        Console.ReadLine();

Using IConnectableObservable seems to lead to Heisenbugs during teardown.

Answer 3

The problem still exists with the current version of the System.Reactive library (5.0.0). I made some progress identifying its cause. The problem is related with the naughty subscribers that omit the onError handler. When this handler is omitted, the Rx puts something like this in its place:

void OnError(Exception error) => throw error;

The error is just rethrown synchronously on the thread that invoked the handler! This has the implication that no other subscriber after the naughty one will receive the OnError notification, because an observable notifies all of its subscribers sequentially and synchronously.

var subject = new Subject<Unit>();
subject.Subscribe(); // Naughty subscriber
subject.OnError(new ApplicationException("@")); // Throws synchronously

(Try it on Fiddle)

Normally this is not a problem, because most notifications coming from built-in Rx operators are invoked asynchronously from ThreadPool threads. This means that there is no way for the client code to catch the exceptions caused by naughty subscribers. And so the exceptions remain unhandled, and are crashing the process. Which is not an ideal behavior for a commercial application, but it is surely much nicer than a hanging process, waiting for a notification that will never come. I am saying this to make it clear that the problem we are trying to solve here is how to make the application crash consistently. When an error occurs and we have a naughty subscriber, crashing the process is the desired behavior. So let's find a way to make it happen with the Observable.Create too.

The problem with the native Observable.Create(Func<IObserver<TResult>, Task>) method is that invoking the asynchronous lambda produces a Task, and the tasks have embedded error handling features. So an exception inside the lambda doesn't escalate automatically to an unhandled exception that crashes the process. My first attempt to fix the Observable.Create method was to await the task, catch the exception, try to propagate it again through the observer.OnError method, and if this attempt failed then rethrow the exception on the ThreadPool and crash the process for good. But this idea is flawed because only the first invocation of the observer.OnError throws. Subsequent invocations are doing nothing, because the built-in observers respect the Rx contract, which disallows multiple OnError notifications. So the try/catch must target specifically the observer.OnError invocation, not the whole asynchronous lambda in general. Which means that we need a wrapper of the supplied observer. The implementation below is based on this idea:

public static IObservable<TSource> CreateObservableEx<TSource>(
    Func<IObserver<TSource>, CancellationToken, Task> subscribeAsync)
{
    return Observable.Create<TSource>(async (observer, cancellationToken) =>
    {
        var innerObserver = Observer.Create<TSource>(observer.OnNext,
            PropagateError, observer.OnCompleted);

        try { await subscribeAsync(innerObserver, cancellationToken); }
        catch (Exception ex)
        {
            PropagateError(ex);
        }

        void PropagateError(Exception error)
        {
            try { observer.OnError(error); }
            catch (Exception ex)
            {
                ThreadPool.QueueUserWorkItem(_ => ExceptionDispatchInfo.Throw(ex));
            }
        }
    });
}

Usage example:

var xs = CreateObservableEx<Unit>(async (o, ct) =>
{
    await Task.Delay(10, ct);
    o.OnError(new Exception());
}).Replay().RefCount();

xs.Subscribe(x => Console.WriteLine(x));
xs.Subscribe(x => Console.WriteLine(x), ex => Console.WriteLine(ex.Message));
await xs.DefaultIfEmpty();

(Try it on Fiddle)

Output:

System.Exception (Unhandled)