ok2c commented on code in PR #789: URL: https://github.com/apache/httpcomponents-client/pull/789#discussion_r2702254222
########## httpclient5/src/main/java/org/apache/hc/client5/http/impl/IdleConnectionEvictor.java: ########## Review Comment: > I have a suggestion here. An issue with this code is that it never evicts connections until they've already been sitting expired in the pool, which creates a window of opportunity for stale connection reuse. > > Instead of passing in `maxIdleTime`, I would pass in `maxIdleTime - localSleepTime`. This will cause all connections to be closed that will exceed their `maxIdleTime` during the next "tick" (where `localSleepTime` is the duration of a tick). This preserves the invariant that the conn pool never leases a connection that has been idle longer than `maxIdleTime`. @rschmitt What should happen if the user chooses sleep interval greater than maxIdleTime? The purpose of this class is not to eliminate the possibility of stale connection reuse but to ensure connections do not stay stuck in the pool if there is no normal activity. That is it. -- 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]
