HTTPCLIENT-677: don't use interrupt for regular wakeup
git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@607293 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
parent
a453692c09
commit
7a2a85f25e
|
@ -1,6 +1,10 @@
|
|||
Changes since 4.0 Alpha 2
|
||||
-------------------
|
||||
|
||||
* [HTTPCLIENT-677] Thread safe client connection manager no longer uses
|
||||
Thread.interrupt() when a connection becomes available.
|
||||
Contributed by Roland Weber <rolandw at apache.org>
|
||||
|
||||
* [HTTPCLIENT-716] Allow application-defined routes.
|
||||
Contributed by Roland Weber <rolandw at apache.org>
|
||||
|
||||
|
|
|
@ -71,10 +71,6 @@ public class ConnPoolByRoute extends AbstractConnPool {
|
|||
private final Log LOG = LogFactory.getLog(ConnPoolByRoute.class);
|
||||
|
||||
|
||||
/** Temporary hack: @@@ a global condition that goes with the lock. */
|
||||
protected final Condition poolCondition;
|
||||
|
||||
|
||||
/** The list of free connections */
|
||||
private Queue<BasicPoolEntry> freeConnections;
|
||||
|
||||
|
@ -98,8 +94,6 @@ public class ConnPoolByRoute extends AbstractConnPool {
|
|||
public ConnPoolByRoute(ClientConnectionManager mgr) {
|
||||
super(mgr);
|
||||
|
||||
poolCondition = poolLock.newCondition(); //@@@ temporary hack
|
||||
|
||||
//@@@ use factory method, at least for waitingThreads
|
||||
freeConnections = new LinkedList<BasicPoolEntry>();
|
||||
waitingThreads = new LinkedList<WaitingThread>();
|
||||
|
@ -222,6 +216,7 @@ public class ConnPoolByRoute extends AbstractConnPool {
|
|||
// TODO: keep track of which routes have waiting threads,
|
||||
// so they avoid being sacrificed before necessary
|
||||
|
||||
boolean success = false;
|
||||
try {
|
||||
if (useTimeout && timeToWait <= 0) {
|
||||
throw new ConnectionPoolTimeoutException
|
||||
|
@ -233,12 +228,9 @@ public class ConnPoolByRoute extends AbstractConnPool {
|
|||
}
|
||||
|
||||
if (waitingThread == null) {
|
||||
//@@@ use factory method?
|
||||
waitingThread = new WaitingThread
|
||||
(poolLock.newCondition(), rospl);
|
||||
//@@@waitingThread.pool = rospl;
|
||||
//@@@waitingThread.thread = Thread.currentThread();
|
||||
} else {
|
||||
waitingThread.interruptedByConnectionPool = false;//@@@
|
||||
}
|
||||
|
||||
if (useTimeout) {
|
||||
|
@ -247,21 +239,17 @@ public class ConnPoolByRoute extends AbstractConnPool {
|
|||
|
||||
rospl.queueThread(waitingThread);
|
||||
waitingThreads.add(waitingThread);
|
||||
//@@@ poolCondition.await(timeToWait, TimeUnit.MILLISECONDS);
|
||||
waitingThread.await(timeToWait); //@@@, TimeUnit.MILLISECONDS);
|
||||
success = waitingThread.await(timeToWait); //@@@, TimeUnit.MILLISECONDS);
|
||||
//@@@ The 'success' flag is somewhat different from the
|
||||
//@@@ previous technique using interrupts. If the CM is
|
||||
//@@@ shutting down, we now get an InterruptedException
|
||||
//@@@ and have no check for that special case. What we
|
||||
//@@@ want to do is to let the exception fly through.
|
||||
//@@@ Actually, we may want to have a special exception
|
||||
//@@@ for the shutdown case, but that is goldplating.
|
||||
|
||||
} catch (InterruptedException e) {
|
||||
if (!waitingThread.interruptedByConnectionPool) {
|
||||
LOG.debug("Interrupted while waiting for connection.", e);
|
||||
throw e;
|
||||
}
|
||||
// Else, do nothing, we were interrupted by the
|
||||
// connection pool and should now have a connection
|
||||
// waiting for us. Continue in the loop and get it.
|
||||
// Or else we are shutting down, which is also
|
||||
// detected in the loop.
|
||||
} finally {
|
||||
if (!waitingThread.interruptedByConnectionPool) {
|
||||
if (!success) {
|
||||
// Either we timed out, experienced a
|
||||
// "spurious wakeup", or were interrupted by an
|
||||
// external thread. Regardless we need to
|
||||
|
@ -269,6 +257,9 @@ public class ConnPoolByRoute extends AbstractConnPool {
|
|||
rospl.removeThread(waitingThread);
|
||||
waitingThreads.remove(waitingThread);
|
||||
}
|
||||
// In case of 'success', we were woken up by the
|
||||
// connection pool and should now have a connection
|
||||
// waiting for us. Continue in the loop and get it.
|
||||
|
||||
if (useTimeout) {
|
||||
endWait = System.currentTimeMillis();
|
||||
|
@ -531,8 +522,7 @@ public class ConnPoolByRoute extends AbstractConnPool {
|
|||
}
|
||||
|
||||
if (waitingThread != null) {
|
||||
waitingThread.interruptedByConnectionPool = true;
|
||||
waitingThread.getThread().interrupt(); //@@@ HTTPCLIENT-677
|
||||
waitingThread.wakeup();
|
||||
}
|
||||
|
||||
} finally {
|
||||
|
@ -586,8 +576,7 @@ public class ConnPoolByRoute extends AbstractConnPool {
|
|||
while (iwth.hasNext()) {
|
||||
WaitingThread waiter = iwth.next();
|
||||
iwth.remove();
|
||||
waiter.interruptedByConnectionPool = true;
|
||||
waiter.getThread().interrupt(); //@@@ HTTPCLIENT-677
|
||||
waiter.getThread().interrupt();
|
||||
}
|
||||
|
||||
routeToPool.clear();
|
||||
|
|
|
@ -60,18 +60,6 @@ public class WaitingThread {
|
|||
private Thread waiter;
|
||||
|
||||
|
||||
/**
|
||||
* Indicates the source of an interruption.
|
||||
* Set to <code>true</code> inside
|
||||
* {@link #notifyWaitingThread(RouteSpecificPool)}
|
||||
* and {@link #shutdown shutdown()}
|
||||
* before the thread is interrupted.
|
||||
* If not set, the thread was interrupted from the outside.
|
||||
*/
|
||||
//@@@ to be removed in HTTPCLIENT-677
|
||||
/*default@@@*/ boolean interruptedByConnectionPool;
|
||||
|
||||
|
||||
/**
|
||||
* Creates a new entry for a waiting thread.
|
||||
*
|
||||
|
@ -99,9 +87,17 @@ public class WaitingThread {
|
|||
*
|
||||
* @param timeout the timeout in milliseconds, or 0 for no timeout
|
||||
*
|
||||
* @return <code>true</code> if the condition was satisfied,
|
||||
* <code>false</code> in case of a timeout.
|
||||
* Typically, a call to {@link #wakeup} is used to indicate
|
||||
* that the condition was satisfied. Since the condition can
|
||||
* be accessed from outside, this cannot be guaranteed though.
|
||||
*
|
||||
* @throws InterruptedException if the waiting thread was interrupted
|
||||
*
|
||||
* @see #wakeup
|
||||
*/
|
||||
public void await(long timeout)
|
||||
public boolean await(long timeout)
|
||||
throws InterruptedException {
|
||||
|
||||
//@@@ check timeout for negative, or assume overflow?
|
||||
|
@ -118,11 +114,14 @@ public class WaitingThread {
|
|||
|
||||
this.waiter = Thread.currentThread();
|
||||
|
||||
boolean success = false;
|
||||
try {
|
||||
this.cond.await(timeout, TimeUnit.MILLISECONDS);
|
||||
success = this.cond.await(timeout, TimeUnit.MILLISECONDS);
|
||||
} finally {
|
||||
this.waiter = null;
|
||||
}
|
||||
return success;
|
||||
|
||||
} // await
|
||||
|
||||
|
||||
|
|
Loading…
Reference in New Issue