hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rola...@apache.org
Subject svn commit: r607293 - in /httpcomponents/httpclient/trunk: RELEASE_NOTES.txt module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java
Date Fri, 28 Dec 2007 17:25:24 GMT
Author: rolandw
Date: Fri Dec 28 09:25:23 2007
New Revision: 607293

URL: http://svn.apache.org/viewvc?rev=607293&view=rev
Log:
HTTPCLIENT-677: don't use interrupt for regular wakeup

Modified:
    httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java

Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=607293&r1=607292&r2=607293&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Fri Dec 28 09:25:23 2007
@@ -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>
 

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java?rev=607293&r1=607292&r2=607293&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
Fri Dec 28 09:25:23 2007
@@ -71,10 +71,6 @@
     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 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 @@
                     // 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 @@
                         }
    
                         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 @@
 
                         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 @@
                             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 @@
             }
 
             if (waitingThread != null) {
-                waitingThread.interruptedByConnectionPool = true;
-                waitingThread.getThread().interrupt(); //@@@ HTTPCLIENT-677
+                waitingThread.wakeup();
             }
 
         } finally {
@@ -586,8 +576,7 @@
             while (iwth.hasNext()) {
                 WaitingThread waiter = iwth.next();
                 iwth.remove();
-                waiter.interruptedByConnectionPool = true;
-                waiter.getThread().interrupt(); //@@@ HTTPCLIENT-677
+                waiter.getThread().interrupt();
             }
 
             routeToPool.clear();

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java?rev=607293&r1=607292&r2=607293&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java
Fri Dec 28 09:25:23 2007
@@ -61,18 +61,6 @@
 
 
     /**
-     * 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.
      *
      * @param cond      the condition for which to wait
@@ -99,9 +87,17 @@
      *
      * @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 @@
 
         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
 
 



Mime
View raw message