Properly integrate async API with server-side cancellations.#5572
Properly integrate async API with server-side cancellations.#5572sreecha merged 3 commits intogrpc:masterfrom
Conversation
There is a comment above IsCancelled that says when it is ok to use this.
| if (server_try_cancel == CANCEL_BEFORE_PROCESSING) { | ||
| ServerTryCancel(&srv_ctx); | ||
| srv_ctx.TryCancel(); | ||
| Verifier(GetParam()).Expect(11, true).Verify(cq_.get()); |
There was a problem hiding this comment.
nit: move the auto verif = Verifier(getParam()) line before this - so that you can use the verif variable
There was a problem hiding this comment.
I thought about that, but I also thought that I'd leave it alone for the following reason. This case is the one where the cancel happens before processing, so I thought I'd start over with a clean verifier after it to insure that we're not leaking any state from this part of the test to the next. Is there a strong reason to do it the way you've suggested?
There was a problem hiding this comment.
Ok. No strong reason. Just that one might wonder why this is being treated as a special case :).
Actually the CANCEL_*_PROCESSING states are all mutually exclusive.. So there shouldn't be any sharing of state between those cases. I don't feel strongly about and its up to you..
|
Looks good to me. The test failures are unrelated to your change. |
Properly integrate async API with server-side cancellations.
|
Which test failure was this fixing? |
There is a comment above IsCancelled that says when it is ok to use this.
Would also appreciate eyes from @ctiller