From 2587d929270416f34ce078823291d34fc6655025 Mon Sep 17 00:00:00 2001 From: Federico Alterio Date: Fri, 11 Oct 2024 14:37:44 +0200 Subject: [PATCH 1/2] Fix ToObservable operator can throw unhandled exception --- .../System/Linq/Operators/ToObservable.cs | 27 ++++++++ .../System/Linq/Operators/ToObservable.cs | 62 ++++++++++++------- 2 files changed, 67 insertions(+), 22 deletions(-) diff --git a/Ix.NET/Source/System.Linq.Async.Tests/System/Linq/Operators/ToObservable.cs b/Ix.NET/Source/System.Linq.Async.Tests/System/Linq/Operators/ToObservable.cs index fa7677ee9..ea8757a63 100644 --- a/Ix.NET/Source/System.Linq.Async.Tests/System/Linq/Operators/ToObservable.cs +++ b/Ix.NET/Source/System.Linq.Async.Tests/System/Linq/Operators/ToObservable.cs @@ -329,6 +329,33 @@ public void ToObservable_SupportsLargeEnumerable() Assert.False(fail); } + [Fact] + public void ToObservable_ShouldForwardExceptionOnGetEnumeratorAsync() + { + var exception = new Exception("Exception message"); + Exception? recievedException = null; + var enumerable = AsyncEnumerable.Create(_ => throw exception); + using var evt = new ManualResetEvent(false); + + var observable = enumerable.ToObservable(); + observable.Subscribe(new MyObserver(_ => + { + evt.Set(); + }, + e => + { + recievedException = e; + evt.Set(); + }, () => + { + evt.Set(); + })); + + evt.WaitOne(); + Assert.NotNull(recievedException); + Assert.Equal(exception.Message, recievedException!.Message); + } + private sealed class MyObserver : IObserver { private readonly Action _onNext; diff --git a/Ix.NET/Source/System.Linq.Async/System/Linq/Operators/ToObservable.cs b/Ix.NET/Source/System.Linq.Async/System/Linq/Operators/ToObservable.cs index 0e60ff045..cdbe1b1d0 100644 --- a/Ix.NET/Source/System.Linq.Async/System/Linq/Operators/ToObservable.cs +++ b/Ix.NET/Source/System.Linq.Async/System/Linq/Operators/ToObservable.cs @@ -38,39 +38,57 @@ public IDisposable Subscribe(IObserver observer) async void Core() { - await using var e = _source.GetAsyncEnumerator(ctd.Token); - do + IAsyncEnumerator e; + + try + { + e = _source.GetAsyncEnumerator(ctd.Token); + } + catch (Exception ex) { - bool hasNext; - var value = default(T)!; + if (!ctd.Token.IsCancellationRequested) + { + observer.OnError(ex); + } - try + return; + } + + await using (e) + { + do { - hasNext = await e.MoveNextAsync().ConfigureAwait(false); - if (hasNext) + bool hasNext; + var value = default(T)!; + + try { - value = e.Current; + hasNext = await e.MoveNextAsync().ConfigureAwait(false); + if (hasNext) + { + value = e.Current; + } } - } - catch (Exception ex) - { - if (!ctd.Token.IsCancellationRequested) + catch (Exception ex) { - observer.OnError(ex); + if (!ctd.Token.IsCancellationRequested) + { + observer.OnError(ex); + } + + return; } - return; - } + if (!hasNext) + { + observer.OnCompleted(); + return; + } - if (!hasNext) - { - observer.OnCompleted(); - return; + observer.OnNext(value); } - - observer.OnNext(value); + while (!ctd.Token.IsCancellationRequested); } - while (!ctd.Token.IsCancellationRequested); } // Fire and forget From 7259cabf5456a21462caea8cb81da8e5d5405db2 Mon Sep 17 00:00:00 2001 From: Federico Alterio Date: Fri, 11 Oct 2024 18:20:39 +0200 Subject: [PATCH 2/2] Unhandled exceptions thrown in DisposeAsync routed to TaskScheduler.UnobservedTaskException --- .../System/Linq/Operators/ToObservable.cs | 109 +++++++++++++----- .../System/Linq/Operators/ToObservable.cs | 55 ++++++++- 2 files changed, 128 insertions(+), 36 deletions(-) diff --git a/Ix.NET/Source/System.Linq.Async.Tests/System/Linq/Operators/ToObservable.cs b/Ix.NET/Source/System.Linq.Async.Tests/System/Linq/Operators/ToObservable.cs index ea8757a63..a53f49267 100644 --- a/Ix.NET/Source/System.Linq.Async.Tests/System/Linq/Operators/ToObservable.cs +++ b/Ix.NET/Source/System.Linq.Async.Tests/System/Linq/Operators/ToObservable.cs @@ -19,14 +19,16 @@ public void ToObservable_Null() Assert.Throws(() => AsyncEnumerable.ToObservable(null)); } - [Fact] - public void ToObservable1() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable1(bool ignoreExceptionsAfterUnsubscribe) { using var evt = new ManualResetEvent(false); var fail = false; - var xs = AsyncEnumerable.Empty().ToObservable(); + var xs = AsyncEnumerable.Empty().ToObservable(ignoreExceptionsAfterUnsubscribe); xs.Subscribe(new MyObserver( x => { @@ -47,15 +49,17 @@ public void ToObservable1() Assert.False(fail); } - [Fact] - public void ToObservable2() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable2(bool ignoreExceptionsAfterUnsubscribe) { using var evt = new ManualResetEvent(false); var lst = new List(); var fail = false; - var xs = Return42.ToObservable(); + var xs = Return42.ToObservable(ignoreExceptionsAfterUnsubscribe); xs.Subscribe(new MyObserver( x => { @@ -77,15 +81,17 @@ public void ToObservable2() Assert.True(lst.SequenceEqual([42])); } - [Fact] - public void ToObservable3() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable3(bool ignoreExceptionsAfterUnsubscribe) { using var evt = new ManualResetEvent(false); var lst = new List(); var fail = false; - var xs = AsyncEnumerable.Range(0, 10).ToObservable(); + var xs = AsyncEnumerable.Range(0, 10).ToObservable(ignoreExceptionsAfterUnsubscribe); xs.Subscribe(new MyObserver( x => { @@ -107,8 +113,10 @@ public void ToObservable3() Assert.True(lst.SequenceEqual(Enumerable.Range(0, 10))); } - [Fact] - public void ToObservable_ThrowOnMoveNext() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable_ThrowOnMoveNext(bool ignoreExceptionsAfterUnsubscribe) { using var evt = new ManualResetEvent(false); @@ -116,7 +124,7 @@ public void ToObservable_ThrowOnMoveNext() var ex_ = default(Exception); var fail = false; - var xs = Throw(ex1).ToObservable(); + var xs = Throw(ex1).ToObservable(ignoreExceptionsAfterUnsubscribe); xs.Subscribe(new MyObserver( x => { @@ -139,8 +147,10 @@ public void ToObservable_ThrowOnMoveNext() Assert.Equal(ex1, ex_); } - [Fact] - public void ToObservable_ThrowOnCurrent() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable_ThrowOnCurrent(bool ignoreExceptionsAfterUnsubscribe) { var ex1 = new Exception("Bang!"); var ex_ = default(Exception); @@ -150,7 +160,7 @@ public void ToObservable_ThrowOnCurrent() _ => new ThrowOnCurrentAsyncEnumerator(ex1) ); - ae.ToObservable() + ae.ToObservable(ignoreExceptionsAfterUnsubscribe) .Subscribe(new MyObserver( x => { @@ -170,8 +180,10 @@ public void ToObservable_ThrowOnCurrent() Assert.Equal(ex1, ex_); } - [Fact] - public void ToObservable_DisposesEnumeratorOnCompletion() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable_DisposesEnumeratorOnCompletion(bool ignoreExceptionsAfterUnsubscribe) { using var evt = new ManualResetEvent(false); @@ -184,7 +196,7 @@ public void ToObservable_DisposesEnumeratorOnCompletion() () => { evt.Set(); return default; })); ae - .ToObservable() + .ToObservable(ignoreExceptionsAfterUnsubscribe) .Subscribe(new MyObserver( x => { @@ -203,8 +215,10 @@ public void ToObservable_DisposesEnumeratorOnCompletion() Assert.False(fail); } - [Fact] - public void ToObservable_DisposesEnumeratorWhenSubscriptionIsDisposed() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable_DisposesEnumeratorWhenSubscriptionIsDisposed(bool ignoreExceptionsAfterUnsubscribe) { using var evt = new ManualResetEvent(false); @@ -227,7 +241,7 @@ public void ToObservable_DisposesEnumeratorWhenSubscriptionIsDisposed() })); subscription = ae - .ToObservable() + .ToObservable(ignoreExceptionsAfterUnsubscribe) .Subscribe(new MyObserver( x => { @@ -250,8 +264,10 @@ public void ToObservable_DisposesEnumeratorWhenSubscriptionIsDisposed() Assert.False(fail); } - [Fact] - public void ToObservable_DesNotCallMoveNextAgainWhenSubscriptionIsDisposed() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable_DesNotCallMoveNextAgainWhenSubscriptionIsDisposed(bool ignoreExceptionsAfterUnsubscribe) { using var evt = new ManualResetEvent(false); @@ -277,7 +293,7 @@ public void ToObservable_DesNotCallMoveNextAgainWhenSubscriptionIsDisposed() })); subscription = ae - .ToObservable() + .ToObservable(ignoreExceptionsAfterUnsubscribe) .Subscribe(new MyObserver( x => { @@ -301,14 +317,16 @@ public void ToObservable_DesNotCallMoveNextAgainWhenSubscriptionIsDisposed() Assert.False(fail); } - [Fact] - public void ToObservable_SupportsLargeEnumerable() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable_SupportsLargeEnumerable(bool ignoreExceptionsAfterUnsubscribe) { using var evt = new ManualResetEvent(false); var fail = false; - var xs = AsyncEnumerable.Range(0, 10000).ToObservable(); + var xs = AsyncEnumerable.Range(0, 10000).ToObservable(ignoreExceptionsAfterUnsubscribe); xs.Subscribe(new MyObserver( x => { @@ -329,15 +347,46 @@ public void ToObservable_SupportsLargeEnumerable() Assert.False(fail); } - [Fact] - public void ToObservable_ShouldForwardExceptionOnGetEnumeratorAsync() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable_ShouldNotCrashOnEnumeratorDispose(bool ignoreExceptionsAfterUnsubscribe) + { + var exception = new Exception("Exception message"); + Exception? received = null; + var enumerable = AsyncEnumerable.Create(_ => throw exception); + using var evt = new ManualResetEvent(false); + + var observable = enumerable.ToObservable(ignoreExceptionsAfterUnsubscribe); + observable.Subscribe(new MyObserver(_ => + { + evt.Set(); + }, + e => + { + received = e; + evt.Set(); + }, () => + { + evt.Set(); + })); + + evt.WaitOne(); + Assert.NotNull(received); + Assert.Equal(exception.Message, received!.Message); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ToObservable_ShouldForwardExceptionOnGetEnumeratorAsync(bool ignoreExceptionsAfterUnsubscribe) { var exception = new Exception("Exception message"); Exception? recievedException = null; var enumerable = AsyncEnumerable.Create(_ => throw exception); using var evt = new ManualResetEvent(false); - var observable = enumerable.ToObservable(); + var observable = enumerable.ToObservable(ignoreExceptionsAfterUnsubscribe); observable.Subscribe(new MyObserver(_ => { evt.Set(); diff --git a/Ix.NET/Source/System.Linq.Async/System/Linq/Operators/ToObservable.cs b/Ix.NET/Source/System.Linq.Async/System/Linq/Operators/ToObservable.cs index cdbe1b1d0..87ab3e63c 100644 --- a/Ix.NET/Source/System.Linq.Async/System/Linq/Operators/ToObservable.cs +++ b/Ix.NET/Source/System.Linq.Async/System/Linq/Operators/ToObservable.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Collections.Generic; +using System.Threading.Tasks; namespace System.Linq { @@ -20,23 +21,44 @@ public static IObservable ToObservable(this IAsyncEnumerable(source); + return new ToObservableObservable(source, false); } + + /// + /// Converts an async-enumerable sequence to an observable sequence. + /// + /// The type of the elements in the source sequence. + /// Enumerable sequence to convert to an observable sequence. + /// If this is true, exceptions that occur after all observers have unsubscribed will be handled and silently ignored. + /// If false, they will go unobserved, meaning they will eventually emerge through + /// The observable sequence whose elements are pulled from the given enumerable sequence. + /// is null. + public static IObservable ToObservable(this IAsyncEnumerable source, bool ignoreExceptionsAfterUnsubscribe) + { + if (source == null) + throw Error.ArgumentNull(nameof(source)); + + return new ToObservableObservable(source, ignoreExceptionsAfterUnsubscribe); + } + + private sealed class ToObservableObservable : IObservable { private readonly IAsyncEnumerable _source; + private readonly bool _ignoreExceptionsAfterUnsubscribe; - public ToObservableObservable(IAsyncEnumerable source) + public ToObservableObservable(IAsyncEnumerable source, bool ignoreExceptionsAfterUnsubscribe) { _source = source; + _ignoreExceptionsAfterUnsubscribe = ignoreExceptionsAfterUnsubscribe; } public IDisposable Subscribe(IObserver observer) { var ctd = new CancellationTokenDisposable(); - async void Core() + async ValueTask Core() { IAsyncEnumerator e; @@ -54,7 +76,8 @@ async void Core() return; } - await using (e) + + try { do { @@ -86,13 +109,33 @@ async void Core() } observer.OnNext(value); + } while (!ctd.Token.IsCancellationRequested); + } + finally + { + if (_ignoreExceptionsAfterUnsubscribe) + { + try + { + await e.DisposeAsync().ConfigureAwait(false); + } + catch + { + // Ignored + } + } + else + { + // Exceptions will go in TaskScheduler.UnobservedTaskException. + // This behavior is similar to Observable.FromAsync + + await e.DisposeAsync().ConfigureAwait(false); } - while (!ctd.Token.IsCancellationRequested); } } // Fire and forget - Core(); + _ = Core(); return ctd; }