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

[reactor-grpc] Cancel signal should be ignored once Complete has been seen #194

Open
ericbottard opened this issue Oct 1, 2019 · 7 comments
Labels

Comments

@ericbottard
Copy link

Per the reactive-streams spec items 1.6 and 3.7 combined, once a stream is complete, any cancel signal should be ignored.

This is not the case with reactor-grpc, which can have drastic effects as we saw in the following setup: Use of the groupBy() operator, which happens to cancel() in addition to regular complete(), on an rpc input Flux led to the cancellation of the output Flux as well, which is not intended.

@bsideup
Copy link
Contributor

bsideup commented Oct 1, 2019

cc @OlegDokuka

@rmichela
Copy link
Collaborator

rmichela commented Oct 1, 2019

Thanks for the report @ericbottard. Would it be possible to provide a test case?

@ericbottard
Copy link
Author

Sadly I don't think I have a simple to reproduce test-case for you. You can have a look at the implementation in the linked PR above, but you may not be able to make much sense of it.

To reproduce simply, you'd need to rpc invoke a

Flux<O> fn(Flux<I> input)

function where:

  • the function continues to emit on its Flux<O> output after the input completes (or cancels, for that matter). It could for example disregard its input entirely and use Flux.interval()
  • have the Flux<I> input complete() AND cancel(), as is done for example by FluxGroupBy
  • this will freak out the gRPC machinery, which will cancel() the output stream as well.

@rmichela
Copy link
Collaborator

rmichela commented Oct 1, 2019

this will freak out the gRPC machinery

That can happen. While gRPC functions like a reactive stream most of the time, it does not comply with many of the rules of the reactive-streams spec, especially around stream cancellation.

@rmichela rmichela added the bug label Oct 2, 2019
@rmichela
Copy link
Collaborator

rmichela commented Oct 2, 2019

@ericbottard Were you able to find a work around, or is this issue a blocker for you?

@ericbottard
Copy link
Author

@rmichela We've got a workaround for now, namely bridging the reactive-grpc Flux with one that won't forward cancels after complete.

@rmichela
Copy link
Collaborator

rmichela commented May 7, 2020

Closing due to lack of repro.

@rmichela rmichela closed this as completed May 7, 2020
@rmichela rmichela reopened this May 7, 2020
@rmichela rmichela added reactor and removed reactor labels Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants