rschmitt commented on code in PR #543:
URL:
https://github.com/apache/httpcomponents-core/pull/543#discussion_r2261231341
##########
httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/AbstractHttp1StreamDuplexer.java:
##########
@@ -277,13 +278,14 @@ public final void onInput(final ByteBuffer src) throws
HttpException, IOExceptio
return;
}
- boolean endOfStream = false;
if (incomingMessage == null) {
final int bytesRead = inbuf.fill(ioSession);
Review Comment:
I think there's a key issue right here: we can't read bytes _and _
`endOfStream` in the same operation. So even if the closure of the connection
is already known to the host (at some level of abstraction), this code doesn't
learn about it until the next event loop iteration, by which point the
connection has already been returned to the pool and potentially reused.
I guess one question here is: should we read in a loop _until_ `bytesRead`
comes back as either 0 or -1? Kind of an edge-triggered approach, where you
drain the file descriptor until `read()` returns would-block (`EAGAIN`). If
possible, I think that approach, combined with this change set, would greatly
reduce or eliminate purely internal race conditions in the client's connection
management.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org
