-
Notifications
You must be signed in to change notification settings - Fork 181
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
SplittingWriteEventsListener
expects that an async source limits recursion depth to 1
#1652
Labels
bug
Something isn't working
Comments
idelpivnitskiy
added a commit
to idelpivnitskiy/servicetalk
that referenced
this issue
Jul 2, 2021
Motivation: To mitigate apple#1652 issue, temporary limit the mutual recursion between `onNext` and `request` to a depth of 1. Modifications: - Use the left 4 bits of the `state` to limit recursion depth to 1; - Remove `boundedDepthOfOnNextAndRequestRecursion()` override from `PublisherFrom2TckTest` and `PublisherFrom3TckTest`; Result: 1. `FromNPublisher` has maximum recursion depth of 1. 2. Less risk encountering the issue apple#1652. Signed-off-by: Idel Pivnitskiy <[email protected]>
idelpivnitskiy
added a commit
to idelpivnitskiy/servicetalk
that referenced
this issue
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 maximum recursion depth of 1. 2. Less risk encountering the issue apple#1652. Signed-off-by: Idel Pivnitskiy <[email protected]>
idelpivnitskiy
added a commit
that referenced
this issue
Jul 2, 2021
Motivation: To mitigate #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 #1652.
idelpivnitskiy
added a commit
that referenced
this issue
Jul 2, 2021
Motivation: To mitigate #1652 issue, temporary limit the mutual recursion between `onNext` and `request` to a depth of 1. Modifications: - Use the left 4 bits of the `state` to limit recursion depth to 1; - Remove `boundedDepthOfOnNextAndRequestRecursion()` override from `PublisherFrom2TckTest` and `PublisherFrom3TckTest`; Result: 1. `FromNPublisher` has a maximum recursion depth of 1. 2. Less risk of encountering issue #1652.
bondolo
pushed a commit
to bondolo/servicetalk
that referenced
this issue
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 issue
Jul 2, 2021
Motivation: To mitigate apple#1652 issue, temporary limit the mutual recursion between `onNext` and `request` to a depth of 1. Modifications: - Use the left 4 bits of the `state` to limit recursion depth to 1; - Remove `boundedDepthOfOnNextAndRequestRecursion()` override from `PublisherFrom2TckTest` and `PublisherFrom3TckTest`; Result: 1. `FromNPublisher` 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 issue
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 issue
Jul 2, 2021
Motivation: To mitigate apple#1652 issue, temporary limit the mutual recursion between `onNext` and `request` to a depth of 1. Modifications: - Use the left 4 bits of the `state` to limit recursion depth to 1; - Remove `boundedDepthOfOnNextAndRequestRecursion()` override from `PublisherFrom2TckTest` and `PublisherFrom3TckTest`; Result: 1. `FromNPublisher` 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
SplittingWriteEventsListener
expects the following sequence of HTTP messages:If the source does not limit recursion to a depth of 1 when it delivers data (see PublisherFrom2TckTest and PublisherFrom3TckTest), the sequence of events can be any, depending on how
WriteDemandEstimator
requests items.Steps to reproduce:
WriteDemandEstimators#newDefaultEstimator()
to always return 1L fromestimateRequestN
.ContentLengthAndTrailersTest
By playing with
WriteDemandEstimator
you can achieve any sequence of events. Examples:Because the sequent can be any,
SplittingWriteEventsListener
andCountingFlushStrategyProvider
can not correctly detect message boundaries. As the result, data may hang in the socket buffer and will never be flushed.This is less problematic when task-based offloading is enabled because each onNext will be on a different task thread, but causes issues when offloading is disabled.
Workaround:
flushOnEach()
strategy, which will trigger the flush for any item, regardless of the boundary. Users can opt-out from our internal flush strategy optimizations by using streaming API or applyingtransform
operation on the HTTP message in a filter (it will reset the "aggregated" and "empty" flags).Ways to reduce the likelihood of this issue for
flushOnEnd()
:After this is fixed, revert the following patches:
FromNPublisher
: limit recursion depth to 1 #1653SingleConcatWithPublisher
: limit recursion depth to 1 #1654The text was updated successfully, but these errors were encountered: