[GitHub] [httpcomponents-core] carterkozak commented on a change in pull request #277: Fixes race condition in the connection lease request logic

[GitBox <gi...@apache.org>]

carterkozak commented on a change in pull request #277:
URL: https://github.com/apache/httpcomponents-core/pull/277#discussion_r605194858



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/pool/LaxConnPool.java
##########
@@ -466,7 +469,20 @@ private void removeLeased(final PoolEntry<T, C> entry) {
                 final Timeout requestTimeout,
                 final FutureCallback<PoolEntry<T, C>> callback) {
             Asserts.check(!terminated.get(), "Connection pool shut down");
-            final BasicFuture<PoolEntry<T, C>> future = new BasicFuture<>(callback);
+            final BasicFuture<PoolEntry<T, C>> future = new BasicFuture<PoolEntry<T, C>>(callback) {
+
+                @Override
+                public synchronized PoolEntry<T, C> get(

Review comment:
       Are concurrent `PoolEntry.get` calls allowed? If so, it's unfortunate we can wait longer than `timeout` if another caller is waiting longer due to `synchronized` acquisition having no upper bound. In the case that we don't wait the full `timeout`, we may have waited a large portion of the expected time already, but we reset the timer once the object monitor has been acquired.
   
   To be clear, I think that sort of bug (if it is legitimate) is much more palatable than the resource exhaustion/leak, so I don't think we need to block on 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org