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

fix: finalization/teardown fixes for AsyncGenerators and take #6062

Merged
merged 2 commits into from
Feb 27, 2021

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Feb 26, 2021

  • Resolves an issue where AsyncGenerators were not properly finalized when an observable result was unsubscribed.
  • Resolves another nearly impossible to test scenario that this happens to cover: In the event of an operator like take() completing the resulting observable, if an error is thrown in the terminal completion handler, teardown is not invoked, as it would not continue to the next line to call the unsubscribe method of the Subscriber instance. This was resolved by ensuring synchronous calls before the unsubscribe call are called in a try/finally block.

Fixes #5998

@benlesh benlesh requested a review from cartant February 26, 2021 23:28
- Resolves an issue where AsyncGenerators were not properly finalized when an observable result was unsubscribed.
- Resolves another nearly impossible to test scenario that this happens to cover: In the event of an operator like `take()` completing the resulting observable, if an error is thrown in the terminal completion handler, teardown is not invoked, as it would not continue to the next line to call the unsubscribe method of the Subscriber instance. This was resolved by ensuring synchronous calls before the unsubscribe call are called in a try/finally block.

Fixes ReactiveX#5998
@benlesh benlesh force-pushed the fix/5998-finalize-async-generators branch from faff523 to 44b4b8a Compare February 26, 2021 23:32
Copy link
Collaborator

@cartant cartant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

spec/observables/from-spec.ts Outdated Show resolved Hide resolved
spec/observables/from-spec.ts Outdated Show resolved Hide resolved
@benlesh benlesh merged commit a2b9563 into ReactiveX:master Feb 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

from(AsyncIterable) doesn't finalize async generators
2 participants