You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2009/01/31 08:36:50 UTC

svn commit: r739517 - in /tomcat/tc6.0.x/trunk: ./ STATUS.txt java/org/apache/catalina/core/StandardWrapper.java webapps/docs/changelog.xml

Author: markt
Date: Sat Jan 31 07:36:49 2009
New Revision: 739517

URL: http://svn.apache.org/viewvc?rev=739517&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=45608
Prevent race condition for allocate/deallocate in StandardWrapper

Modified:
    tomcat/tc6.0.x/trunk/   (props changed)
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc6.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Sat Jan 31 07:36:49 2009
@@ -1 +1 @@
-/tomcat/trunk:601180,606992,612607,630314,652744,653247,673796,673820,683982,684001,684081,684234,684269-684270,687503,687645,690781,691392,691805,692748,695053,695311,696780,696782,698012,698227,698236,698613,699427,699634,709294,709811,709816,710063,710066,710125,710205,711126,711600,712461,712467,718360,719602,719626,719628,720046,720069,721040,723404,723738,726052,727303,728032,728768,728947,729057,729567,729569,729809,729815,729934,730250,732859
+/tomcat/trunk:601180,606992,612607,630314,652744,653247,673796,673820,683982,684001,684081,684234,684269-684270,685177,687503,687645,690781,691392,691805,692748,695053,695311,696780,696782,698012,698227,698236,698613,699427,699634,709294,709811,709816,710063,710066,710125,710205,711126,711600,712461,712467,718360,719602,719626,719628,720046,720069,721040,723404,723738,726052,727303,728032,728768,728947,729057,729567,729569,729809,729815,729934,730250,732859

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=739517&r1=739516&r2=739517&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sat Jan 31 07:36:49 2009
@@ -38,14 +38,6 @@
    0: remm (looks risky, very minor problem), fhanik - minor problem
   -1: 
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=45608
-  Prevent race condition for allocate/deallocate in StandardWrapper
-  http://svn.apache.org/viewvc?rev=685177&view=rev
-  +1: markt, funkman, fhanik
-   0: remm: The only unsafe usage of this field is to implement the gimmick loop-wait hack,
-            which to me means adding any sort of sync is not worth it.
-  -1: 
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=45074
   http://svn.apache.org/viewvc?rev=689402&view=rev
   +1: jfclere, funkman, fhanik

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java?rev=739517&r1=739516&r2=739517&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java Sat Jan 31 07:36:49 2009
@@ -28,6 +28,7 @@
 import java.util.HashSet;
 import java.util.Properties;
 import java.util.Stack;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
@@ -135,7 +136,7 @@
      * The count of allocations that are currently active (even if they
      * are for the same instance, as will be true on a non-STM servlet).
      */
-    protected int countAllocated = 0;
+    protected AtomicInteger countAllocated = new AtomicInteger(0);
 
 
     /**
@@ -340,7 +341,7 @@
      */
     public int getCountAllocated() {
 
-        return (this.countAllocated);
+        return (this.countAllocated.get());
 
     }
 
@@ -810,7 +811,7 @@
                             // condition with unload. Bug 43683, test case #3
                             if (!singleThreadModel) {
                                 newInstance = true;
-                                countAllocated++;
+                                countAllocated.incrementAndGet();
                             }
                         } catch (ServletException e) {
                             throw e;
@@ -828,7 +829,7 @@
                 // For new instances, count will have been incremented at the
                 // time of creation
                 if (!newInstance) {
-                    countAllocated++;
+                    countAllocated.incrementAndGet();
                 }
                 return (instance);
             }
@@ -836,7 +837,7 @@
 
         synchronized (instancePool) {
 
-            while (countAllocated >= nInstances) {
+            while (countAllocated.get() >= nInstances) {
                 // Allocate a new instance if possible, or else wait
                 if (nInstances < maxInstances) {
                     try {
@@ -858,7 +859,7 @@
             }
             if (log.isTraceEnabled())
                 log.trace("  Returning allocated STM instance");
-            countAllocated++;
+            countAllocated.incrementAndGet();
             return (Servlet) instancePool.pop();
 
         }
@@ -879,13 +880,13 @@
 
         // If not SingleThreadModel, no action is required
         if (!singleThreadModel) {
-            countAllocated--;
+            countAllocated.decrementAndGet();
             return;
         }
 
         // Unlock and free this instance
         synchronized (instancePool) {
-            countAllocated--;
+            countAllocated.decrementAndGet();
             instancePool.push(servlet);
             instancePool.notify();
         }
@@ -1358,13 +1359,13 @@
 
         // Loaf a while if the current instance is allocated
         // (possibly more than once if non-STM)
-        if (countAllocated > 0) {
+        if (countAllocated.get() > 0) {
             int nRetries = 0;
             long delay = unloadDelay / 20;
-            while ((nRetries < 21) && (countAllocated > 0)) {
+            while ((nRetries < 21) && (countAllocated.get() > 0)) {
                 if ((nRetries % 10) == 0) {
                     log.info(sm.getString("standardWrapper.waiting",
-                                          new Integer(countAllocated)));
+                                          countAllocated.toString()));
                 }
                 try {
                     Thread.sleep(delay);

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=739517&r1=739516&r2=739517&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Sat Jan 31 07:36:49 2009
@@ -134,6 +134,11 @@
         be instantiated. (funkman) 
       </add>
       <fix>
+        <bug>45608</bug>: Make allocated servlet count synchronized to ensure
+        the correct allocated servlet count is available during shutdown.
+        (markt)
+      </fix>
+      <fix>
         <bug>45628</bug>: When checking MANIFEST dependancies, JARs without
         dependencies should allows be considered to be full-filled. (markt) 
       </fix>



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