Respect requested message limits within a single MessageProducer in BinderTransport.#9163
Conversation
markb74
left a comment
There was a problem hiding this comment.
Looks good to me, but wait for ejona since the new test affects other transport as well.
ejona86
left a comment
There was a problem hiding this comment.
I believe flowControlPushBack() would have ordinarily caught this bug, but that test assumes end-to-end flow control which isn't in-place for Binder so the test is disabled. This new test is inherently timing-sensitive (just like flowControlPushBack()) and might not catch bugs. @akandratovich, did you confirm the test fails before the bug fix?
|
@ejona86: I verified it failed before the fix, but that's a good point about timing. It might help to have the client halfClose, and then awaitHalfClose on the server. |
|
Sorry, awaitHalfClose won't help, but adding a doPingPong() call should help. |
|
Ah, yes. |
I tried enabling |
Add check for request message count in binder grpc inbound stream.
Without check if messages are available and requested, producer provides everything that is available ignoring the request count. It leads to call listener API violation - it's possible that more messages will be provided than requested.