Uploaded image for project: 'HttpComponents HttpCore'
  1. HttpComponents HttpCore
  2. HTTPCORE-589

LaxConnPool may not allocate available connection

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Resolved
    • Minor
    • Resolution: Fixed
    • 5.0-beta8
    • 5.0-beta9
    • HttpCore
    • None

    Description

      The LaxConnPool can fail to allocate available connections from the pool in rare circumstances.

      The issue happens when 2 threads execute concurrently; one requesting a lease while another releases a pool entry.

      Thread 1
      future = lease
        getAvailableEntry = null
        allocatePoolEntry = false
        <about to add a LeaseRequest to pending queue>
      Thread 2
      release
        removeLeased
        available.add
        servicePendingRequest
          while (pending.poll() != null)  does nothing because pending queue is empty
        <release call completed, with an entry now in the available list>
      Thread 1
        pending.add(new LeaseRequest())
        <lease call completed, with an unsatisfied future>
      future.get()

      And now Thread 1 is blocked until there is another release call, even though there is actually an available connection in the pool.

      The problem arises because there is a window between checking for an available pool entry and registering on the pending request queue. In that window, the statte of the pool can change.

      Recreating this problem is test is extremely difficult, because the window for failure is really very small. I've only be able to observe the error in running code by introducing synchronization or delay (as small as 3ms, but still needed) into the pool code just before the lease pending.add, so as to ensure the necessary timing condition between threads exist. My test class:

      import org.apache.hc.core5.io.ModalCloseable;
      import org.apache.hc.core5.io.CloseMode;
      import org.apache.hc.core5.pool.PoolEntry;
      import org.apache.hc.core5.pool.LaxConnPool;
      import org.apache.hc.core5.pool.PoolEntry;
      import org.apache.hc.core5.util.TimeValue;
      
      import java.util.concurrent.Future;
      
      
      public class PoolPendTest {
      
        private static class TestConn implements ModalCloseable {
          public void close() {
          }
          public void close(CloseMode mode) {
          }
        }
      
        private static LaxConnPool<String,TestConn> pool;
      
        private static void dbg(String msg) {
          System.out.println(Thread.currentThread().getName()+": "+System.currentTimeMillis()+": "+msg);
        }
      
        public static void main(String[] args) {
          final int poolSize = 1;
          pool = new FixPendPool<>(poolSize);
          // Create a pool entry purely to prime class loading
          new PoolEntry<>("", TimeValue.NEG_ONE_MILLISECONDS);
          
          for (int i=0;i<poolSize*2;i++) {
            new Thread(String.format("req-%04d", i)) {
              @Override
              public void run() {
                dbg("Started");
                Future<PoolEntry<String,TestConn>> req = pool.lease("somehost",null);
                PoolEntry<String,TestConn> entry = null;
                if (req.isDone()) {
                  try {
                    entry = req.get();
                    if (!entry.hasConnection()) {
                      entry.assignConnection(new TestConn());
                      pool.release(entry, true);
                      dbg("Released pool entry "+System.identityHashCode(entry));
                    }
                  }
                  catch (Exception shouldNotHappen) {
                    dbg("Should not happen!");
                    shouldNotHappen.printStackTrace();
                  }
                }
                try {
                  dbg("Assigned pool entry "+System.identityHashCode(req.get()));
                }
                catch (Exception fail) {
                  dbg("Getting pool entry failed");
                  fail.printStackTrace();
                }
              }
            }.start();
          }
        }
      }
      

      When I modify the LaxConnPool code to add a 3ms delay just before adding to the pend queue:

          try { Thread.sleep(3); } catch (InterruptedException onward) {}
          pending.add(new LeaseRequest<>(state, requestTimeout, future));
      

      then the test class blocks with an infinite wait where the pool contains a returned connection, but it's never assigned to the second thread.

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              linton.miller Linton Miller
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 1h 20m
                  1h 20m