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

Fixes requestChannel to ensure support of half-closed state #794

Merged
merged 3 commits into from
Apr 23, 2020

Conversation

OlegDokuka
Copy link
Member

@OlegDokuka OlegDokuka commented Apr 23, 2020

This PR fixes a bug on the requester side and ensures that the requester is capable of handling requests when its publisher has already sent or received completion.

The Bug here is in the fact that RequestChannel on the requester side has 2 maps to track the state:

  • receivers which is there to receive OnNext/OnError/OnComplete signals
  • senders - requestChannel specific map to hold a subscription and receive the request/cancel signals from the remote.

This means if one is absent and another is present - the stream is in the half-closed state.

Originally, we had that -> https://github.com/rsocket/rsocket-java/blob/develop/rsocket-core/src/main/java/io/rsocket/core/RSocketRequester.java#L562

Subscriber<Payload> receiver = receivers.get(streamId);
    if (receiver == null) {
      handleMissingResponseProcessor(streamId, type, frame);
    }

which means that if there is no receiver - it is impossible to handle any frames even though the sender map contains handling subscriptions.

Due to the spec, there is a half-closed state when the responder may send incomplete, but the requester may still continue its stream and send onXXX signals along with being able to receive demand (requestN) and cancel frames.

OUTER PUBLISHER COMPLETED - INNER PUBLISHER CAN SEND
OUTER PUBLISHER CANCELLED - INNER PUBLISHER CAN SEND
INNER PUBLISHER COMPLETED - OUTER PUBLISHER CAN SEND
INNER PUBLISHER CANCELLED - the WHOLE CHAIN IS TERMINATED

Signed-off-by: Oleh Dokuka shadowgun@i.ua

OUTER PUBLISHER COMPLETED - INNER PUBLISHER CAN SEND
OUTER PUBLISHER CANCELLED - INNER PUBLISHER CAN SEND
INNER PUBLISHER COMPLETED - OUTER PUBLISHER CAN SEND
INNER PUBLISHER CANCELLED - WHOLE CHAIN IS TERMINATED

Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka OlegDokuka added the bug label Apr 23, 2020
@OlegDokuka OlegDokuka added this to the 1.0 RC7 milestone Apr 23, 2020
@OlegDokuka OlegDokuka added this to In progress in Core Apr 23, 2020
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@@ -560,46 +560,58 @@ private void handleStreamZero(FrameType type, ByteBuf frame) {

private void handleFrame(int streamId, FrameType type, ByteBuf frame) {
Subscriber<Payload> receiver = receivers.get(streamId);
if (receiver == null) {
Copy link
Member Author

Choose a reason for hiding this comment

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

this statement is basically a bug which prevents handling the requestN frames when the receiver is completed

Copy link
Member Author

Choose a reason for hiding this comment

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

Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka OlegDokuka merged commit 480b501 into develop Apr 23, 2020
@OlegDokuka OlegDokuka deleted the bugfix/request-channel-half-closed-support branch April 23, 2020 10:03
@OlegDokuka OlegDokuka moved this from In progress to Done in Core Apr 23, 2020
@rstoyanchev rstoyanchev changed the title fixes requestChannel and ensure support of half-closed state Fixes requestChannel to ensure support of half-closed state Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Core
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants