Single#concat(Publisher) potential demand deadlock fix
#1646
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: Single#concat(Publisher) may result in deadlock if demand comes after the Single completes, and the next Publisher blocks in its subscribe method. This may happen if blocking APIs are used (e.g. ConnectablePayloadWriter) and the control flow involves Single#concat(Publisher). ServiceTalk currently only uses this operator for use in the Publisher#buffer(..) operator. Modifications: - SingleConcatWithPublisher#request(long) should deliver demand to super class before delivering downstream demand and subscribing. Ordering of downstream signals will be preserved because DelayedCancellableThenSubscription won't propagate demand upstream until onSubscribe is called. Result: If downstream requests sufficient demand it will be available to the concat Publisher as soon as it is subscribed to, and therefore less potential for deadlock when using blocking APIs and Single#concat(Publisher).
idelpivnitskiy
changed the title
Single#concat(Publisher) potential demand deadlock fix
|
I will take this in and adjust #1643. Thank you for the test! |
1 task
bondolo
pushed a commit
to bondolo/servicetalk
that referenced
this pull request
Jul 2, 2021
Motivation: `Single#concat(Publisher)` may result in deadlock if demand comes after the `Single` completes, and the next `Publisher` blocks in its subscribe method. This may happen if blocking APIs are used (e.g. `ConnectablePayloadWriter`) and the control flow involves `Single#concat(Publisher)`. ServiceTalk currently only uses this operator for use in the `Publisher#buffer(..)` operator. Modifications: - `SingleConcatWithPublisher#request(long)` should deliver demand to super class before delivering downstream demand and subscribing. Ordering of downstream signals will be preserved because `DelayedCancellableThenSubscription` won't propagate demand upstream until `onSubscribe` is called. Result: If downstream requests sufficient demand it will be available to the concat `Publisher` as soon as it is subscribed to, and therefore less potential for deadlock when using blocking APIs and `Single#concat(Publisher)`.
bondolo
pushed a commit
to bondolo/servicetalk
that referenced
this pull request
Jul 2, 2021
Motivation: `Single#concat(Publisher)` may result in deadlock if demand comes after the `Single` completes, and the next `Publisher` blocks in its subscribe method. This may happen if blocking APIs are used (e.g. `ConnectablePayloadWriter`) and the control flow involves `Single#concat(Publisher)`. ServiceTalk currently only uses this operator for use in the `Publisher#buffer(..)` operator. Modifications: - `SingleConcatWithPublisher#request(long)` should deliver demand to super class before delivering downstream demand and subscribing. Ordering of downstream signals will be preserved because `DelayedCancellableThenSubscription` won't propagate demand upstream until `onSubscribe` is called. Result: If downstream requests sufficient demand it will be available to the concat `Publisher` as soon as it is subscribed to, and therefore less potential for deadlock when using blocking APIs and `Single#concat(Publisher)`.
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:
Single#concat(Publisher) may result in deadlock if demand comes after
the Single completes, and the next Publisher blocks in its subscribe
method. This may happen if blocking APIs are used (e.g.
ConnectablePayloadWriter) and the control flow involves
Single#concat(Publisher). ServiceTalk currently only uses this operator
for use in the Publisher#buffer(..) operator.
Modifications:
class before delivering downstream demand and subscribing. Ordering of
downstream signals will be preserved because
DelayedCancellableThenSubscription won't propagate demand upstream
until onSubscribe is called.
Result:
If downstream requests sufficient demand it will be available to the
concat Publisher as soon as it is subscribed to, and therefore less
potential for deadlock when using blocking APIs and
Single#concat(Publisher).