rschmitt commented on code in PR #785:
URL:
https://github.com/apache/httpcomponents-client/pull/785#discussion_r2695888959
##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java:
##########
@@ -444,6 +444,8 @@ public ConnectionEndpoint get(
conn.activate();
if (connectionConfig.getSocketTimeout() != null) {
conn.setSocketTimeout(connectionConfig.getSocketTimeout());
+ } else {
+
conn.setSocketTimeout(resolveSocketConfig(route).getSoTimeout());
Review Comment:
> `SocketConfig` defines the initial socket parameters only.
Strictly speaking, this is part of the stated contract for many of the
methods in `SocketConfig`, but not `setSoTimeout`:
* `setSoLinger`: "Determines the default value of the {@link
SocketOptions#SO_LINGER} parameter for newly created sockets."
* `setSoReuseAddress`: "Determines the default value of the {@link
SocketOptions#SO_REUSEADDR} parameter for newly created sockets."
* `setSoTimeout`: "Determines the default socket timeout value for blocking
I/O operations."
The failure to re-set `SocketConfig.soTimeout` on reused connections is a
behavior change in 5.6 that I learned about from a user whose integration tests
started failing. I interpret it as a regression.
> It should have no bearing on the persistent connections once they have
been initialized.
Why is the socket timeout getting changed in the first place? I don't
actually know what's changing it or why.
> This change will also likely make behavior of the classic connection
manager inconsistent with that of the async one.
There was no such regression in the async manager, so there's already an
inconsistency somewhere; the sync manager is screwing with the socket timeout
when the async manager isn't.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
