SingleConcatWithPublisher: limit recursion depth to 1
#1654
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Motivation: To mitigate apple#1652 issue, temporary limit the mutual recursion between `onNext` and `request` to a depth of 1 for `ConcatDeferNextSubscriber`. Modifications: - Introduce additional `SINGLE_DELIVERING` state to understand if the `request` was invoked while we deliver result of the `Single`; - Remove `boundedDepthOfOnNextAndRequestRecursion()` override from `SingleConcatWithPublisherDeferSubscribeTckTest`; Result: 1. `SingleConcatWithPublisher` has maximum recursion depth of 1. 2. Less risk encountering the issue apple#1652. Signed-off-by: Idel Pivnitskiy <[email protected]>
|
This was already reviewed in #1643 (see 6032a41). We agreed with @Scottmitch to move this change into a separate PR to revert it later after #1652 is fixed. Will merge after CI passes all checks. |
bondolo
pushed a commit
to bondolo/servicetalk
that referenced
this pull request
Jul 2, 2021
Motivation: To mitigate apple#1652 issue, temporary limit the mutual recursion between `onNext` and `request` to a depth of 1 for `ConcatDeferNextSubscriber`. Modifications: - Introduce additional `SINGLE_DELIVERING` state to understand if the `request` was invoked while we deliver result of the `Single`; - Remove `boundedDepthOfOnNextAndRequestRecursion()` override from `SingleConcatWithPublisherDeferSubscribeTckTest`; Result: 1. `SingleConcatWithPublisher` has a maximum recursion depth of 1. 2. Less risk of encountering issue apple#1652.
bondolo
pushed a commit
to bondolo/servicetalk
that referenced
this pull request
Jul 2, 2021
Motivation: To mitigate apple#1652 issue, temporary limit the mutual recursion between `onNext` and `request` to a depth of 1 for `ConcatDeferNextSubscriber`. Modifications: - Introduce additional `SINGLE_DELIVERING` state to understand if the `request` was invoked while we deliver result of the `Single`; - Remove `boundedDepthOfOnNextAndRequestRecursion()` override from `SingleConcatWithPublisherDeferSubscribeTckTest`; Result: 1. `SingleConcatWithPublisher` has a maximum recursion depth of 1. 2. Less risk of encountering issue apple#1652.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
To mitigate #1652 issue, temporary limit the mutual recursion between
onNextandrequestto a depth of 1 forConcatDeferNextSubscriber.Modifications:
SINGLE_DELIVERINGstate to understand if therequestwas invoked while we deliver result of theSingle;boundedDepthOfOnNextAndRequestRecursion()override fromSingleConcatWithPublisherDeferSubscribeTckTest;Result:
SingleConcatWithPublisherhas a maximum recursion depth of 1.SplittingWriteEventsListenerexpects that an async source limits recursion depth to 1 #1652.