[
https://issues.apache.org/jira/browse/HTTPCORE-589?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16906701#comment-16906701
]
Linton Miller commented on HTTPCORE-589:
----------------------------------------
Fair enough. I could create a two-valued private enum to replace the boolean,
which would make the method invocations more documentary because you don't have
to find the method definition to see what the boolean means.
The flag controls whether the servicePendingRequests tries to process the
entire pending queue, or quits after allocating a single pool entry. That's
because it's called from 2 different contexts:
- release, where only a single entry has been added to the pool, so there's no
point trying to allocate more than one entry to pending connections.
- enumAvailable, where it may have disposed multiple entries, meaning it should
try and service as many pending requests as it can.
Truth is that trying to service the entire queue is a superset of just
servicing one entry, and there would never be any harm in always trying to
service the entire queue every time (indeed, you could even argue it would be a
safety mechanism against other pooling bugs). The only reason to short-circuit
and quit after one request is just to save a bit of work in the normal workflow
of a release (it avoids popping a request off the pending queue, finding no
available entries, then immediately pushing the request back on pending; it
also avoids possible reshuffling of the pending queue order in a busy system
that might make a thread wait longer than "fair", even though there are no
guarantees of fairness in the pool).
> LaxConnPool may not allocate available connection
> -------------------------------------------------
>
> Key: HTTPCORE-589
> URL: https://issues.apache.org/jira/browse/HTTPCORE-589
> Project: HttpComponents HttpCore
> Issue Type: Bug
> Components: HttpCore
> Affects Versions: 5.0-beta8
> Reporter: Linton Miller
> Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> 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:
> {code:java}
> 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();
> }
> }
> }
> {code}
> When I modify the LaxConnPool code to add a 3ms delay just before adding to
> the pend queue:
> {code:java}
> try { Thread.sleep(3); } catch (InterruptedException onward) {}
> pending.add(new LeaseRequest<>(state, requestTimeout, future));
> {code}
> 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.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
