Ethan Wong

> Can you find an official reference for that second point? > > It seems to be against everything officially written about AsyncStream (also seems very unintuitive that it would...

> rom my understanding observable.values works exactly like publisher.values in Combine - it blocks the current context unless you wrap it in a `Task`. Seems to me like expected behavior,...

> @GetToSet sure - but I think that's an acceptable behavior. It's what a user would expect if they try compare with the Combine implementation. IMO It should be the...

> yncStream calls `continuation.onTermination` **both on completion and cancellation**. The current subscription in `Observable.values` might call `disposable.dispose()` and `continuation.onTermination` twice. > > This PR removes `onDisposed` on the subscriptions inside...

@freak4pc I've updated my PR and now it fixes the first and third point, leaving the blocking behavior(second point) untouched.

> IRT No.1 - I'm a bit worried about turning of TSan In tests here but I've read a lot about issues regarding to concurrency, specifically. Perhaps it is worthwhile...

I've made following changes to this PR: 1. For async sequences that throws, manually throw a CancellationError on cancel (similar to `PrimitiveSequence.value`, #2427). 2. Add tests to cover the changes.

Assigning `iTunesDuration` (``) is the correct approach but it could cause inconsistency in case the duration specified on the feed doesn't match the actual audio file. Currently, we refresh the...

> Or maybe simply have two fields/member: one with metadata duration and one with actual media duration? Probably somewhat less efficient memory wise, but probably the logic to handle it...

> 1-3 looks good IMHO. Not sure about 4 - I think it's sort of an edge case and should happen very rarely. For the 4th point, I think Doughnut...