HttpContentDecoder must continue read when it did not produce any mes…#8922
HttpContentDecoder must continue read when it did not produce any mes…#8922normanmaurer merged 2 commits into4.1from
Conversation
…sage and auto read is false Motivation: When HttpContentDecoder (and so HttpContentDecompressor) does not produce any message we need to make sure it calls ctx.read() if auto read is false to not stale. Modifications: - Keep track if we need to call ctx.read() or not - Add unit test Result: Fixes #8915.
ejona86
left a comment
There was a problem hiding this comment.
Why isn't this part of MessageToMessageDecoder?
| try { | ||
| decodeContent(c, out); | ||
| } finally { | ||
| if (!needRead) { |
There was a problem hiding this comment.
I think this variable handling is inverted. needRead should start as true, and if !out.isEmpty() it should be set to false. Then in channelReadComplete() it should be set to true again.
If a read() causes three ByteBufs to be read (because using DefaultMaxBytesRecvByteBufAllocator), which calls decode() three times, the first two times may not put anything in out but the last decode does place something in out. It seems in that case we shouldn't call read.
But there's also the case where the second bytebuf placed something in out. That seems like it shouldn't call read either.
|
@ejona86 its not mark of |
|
I can confirm too that #8915 is fixed by these changes. I have also understood now how the |
…sage and auto read is false
Motivation:
When HttpContentDecoder (and so HttpContentDecompressor) does not produce any message we need to make sure it calls ctx.read() if auto read is false to not stale.
Modifications:
Result:
Fixes #8915.