Netty HttpProtocol.finishUpdate should offer channel back to pool after markAsDone

[maxpower-artvandelay]

HttpProtocol.finishUpdate attempts to put the channel back into the pool then calls markAsDone. This permits a case were the same channel is selected in the callback which can lead to a "read timeout".

[maxpower-artvandelay]

#845

[slandelle]

Thanks!
Please keep those fixes coming :)

[slandelle]

@maxpower-artvandelay I was too hasty in merging this. It causes other issues and the build to fail. Mostly, it causes to open too many connections when the keep-alive connections are not offered back to the pool fast enough.

Could you elaborate on what you observed, please?

[maxpower-artvandelay]

Hey,
So I was able to reproduce this, though the issue title may not be correct anymore but this causing some read timeouts.

Take a look at the example here:
https://gist.github.com/maxpower-artvandelay/75da9297225980cd1cfb

I make a couple non-blocking calls to a url and then make a subsequent blocking call using get() when onCompleted is called. There seems to be always 1 request that seems to comeback with a timeout. Wireshark seems to point that there is a response to the request. The number of requests seems to vary by machine. You may need to increase the 'numRequests' count if its not seen.

[slandelle]

Your test is broken. You're performing blocking calls in the IO thread. As you're using the channel pool and Netty definitively bind a channel to a thread, there's a chance you pick a channel from the pool that's bound to the very same thread you're running on and blocking, hence a deadlock that will only be lifted by a timeout.

Either do only non blocking calls, or hand over request sending to another dedicated thread pool/executor service.

[maxpower-artvandelay]

Aah yes, you are correct. Thanks for the quick reply!