Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ix: Do not dispose the enumerator while enumerating in the ToObservable operator. #915

Merged
merged 9 commits into from Nov 20, 2019
Expand Up @@ -108,7 +108,7 @@ public void ToObservable3()
}

[Fact]
public void ToObservable4()
public void ToObservable_ThrowOnMoveNext()
{
using var evt = new ManualResetEvent(false);

Expand Down Expand Up @@ -139,6 +139,37 @@ public void ToObservable4()
Assert.Equal(ex1, ex_);
}

[Fact]
public void ToObservable_ThrowOnCurrent()
{
var ex1 = new Exception("Bang!");
var ex_ = default(Exception);
var fail = false;

var ae = AsyncEnumerable.Create(
_ => new ThrowOnCurrentAsyncEnumerator(ex1)
);

ae.ToObservable()
.Subscribe(new MyObserver<int>(
x =>
{
fail = true;
},
ex =>
{
ex_ = ex;
},
() =>
{
fail = true;
}
));

Assert.False(fail);
Assert.Equal(ex1, ex_);
}

[Fact]
public void ToObservable_DisposesEnumeratorOnCompletion()
{
Expand Down Expand Up @@ -289,5 +320,18 @@ public MyObserver(Action<T> onNext, Action<Exception> onError, Action onComplete

public void OnNext(T value) => _onNext(value);
}

private sealed class ThrowOnCurrentAsyncEnumerator : IAsyncEnumerator<int>
{
readonly private Exception _exception;
public ThrowOnCurrentAsyncEnumerator(Exception ex)
{
_exception = ex;
}

public int Current => throw _exception;
public ValueTask DisposeAsync() => default;
public ValueTask<bool> MoveNextAsync() => new ValueTask<bool>(true);
}
}
}
Expand Up @@ -28,51 +28,50 @@ public ToObservableObservable(IAsyncEnumerable<T> source)
public IDisposable Subscribe(IObserver<T> observer)
{
var ctd = new CancellationTokenDisposable();
var e = _source.GetAsyncEnumerator(ctd.Token);

async void Core()
{
bool hasNext;
try
await using (var e = _source.GetAsyncEnumerator(ctd.Token))
quinmars marked this conversation as resolved.
Show resolved Hide resolved
{
hasNext = await e.MoveNextAsync().ConfigureAwait(false);
}
catch (Exception ex)
{
if (!ctd.Token.IsCancellationRequested)
do
{
observer.OnError(ex);
await e.DisposeAsync().ConfigureAwait(false);
}
bool hasNext;
var value = default(T)!;

return;
}
try
{
hasNext = await e.MoveNextAsync().ConfigureAwait(false);
if (hasNext)
{
value = e.Current;
}
}
catch (Exception ex)
{
if (!ctd.Token.IsCancellationRequested)
{
observer.OnError(ex);
}

if (hasNext)
{
observer.OnNext(e.Current);
return;
}

if (!ctd.Token.IsCancellationRequested)
{
Core();
}
if (!hasNext)
{
observer.OnCompleted();
return;
}

// In case cancellation is requested, this could only have happened
// by disposing the returned composite disposable (see below).
// In that case, e will be disposed too, so there is no need to dispose e here.
}
else
{
observer.OnCompleted();
await e.DisposeAsync().ConfigureAwait(false);
observer.OnNext(value);
}
while (!ctd.Token.IsCancellationRequested);
}
}

// Fire and forget
Core();

// REVIEW: Safety of concurrent dispose operation; fire-and-forget nature of dispose?

return Disposable.Create(ctd, Disposable.Create(() => { e.DisposeAsync(); }));
return ctd;
}
}
}
Expand Down