Return-Path: 
 <dev-return-143028-apmail-tomcat-dev-archive=tomcat.apache.org@tomcat.apache.org>
X-Original-To: apmail-tomcat-dev-archive@www.apache.org
Delivered-To: apmail-tomcat-dev-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 0BEE11036B
	for <apmail-tomcat-dev-archive@www.apache.org>;
 Tue,  8 Oct 2013 10:35:09 +0000 (UTC)
Received: (qmail 98007 invoked by uid 500); 8 Oct 2013 10:34:49 -0000
Delivered-To: apmail-tomcat-dev-archive@tomcat.apache.org
Received: (qmail 97811 invoked by uid 500); 8 Oct 2013 10:34:46 -0000
Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@tomcat.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@tomcat.apache.org>
List-Post: <mailto:dev@tomcat.apache.org>
List-Id: <dev.tomcat.apache.org>
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Delivered-To: mailing list dev@tomcat.apache.org
Received: (qmail 97759 invoked by uid 99); 8 Oct 2013 10:34:44 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 08 Oct 2013 10:34:44 +0000
X-ASF-Spam-Status: No, hits=-2000.0 required=5.0
	tests=ALL_TRUSTED
X-Spam-Check-By: apache.org
Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4)
    by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 08 Oct 2013 10:34:41 +0000
Received: from eris.apache.org (localhost [127.0.0.1])
	by eris.apache.org (Postfix) with ESMTP id 358A4238899C
	for <dev@tomcat.apache.org>; Tue,  8 Oct 2013 10:34:20 +0000 (UTC)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1530216 - in /tomcat/tc7.0.x/trunk: ./
 java/org/apache/catalina/loader/WebappClassLoader.java
 webapps/docs/config/context.xml
Date: Tue, 08 Oct 2013 10:34:20 -0000
To: dev@tomcat.apache.org
From: markt@apache.org
X-Mailer: svnmailer-1.0.9
Message-Id: <20131008103420.358A4238899C@eris.apache.org>
X-Virus-Checked: Checked by ClamAV on apache.org

Author: markt
Date: Tue Oct  8 10:34:19 2013
New Revision: 1530216

URL: http://svn.apache.org/r1530216
Log:
Fix an intermittent failure with TestWebappClassLoaderExecutorMemoryLeak.
If the executor threads did not stop quickly enough, Thread.stop() was used. Sometimes this resulted in the executor not shutting down and the test failing. This makes using clearReferencesStopThreads marginally safer.

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
    tomcat/tc7.0.x/trunk/webapps/docs/config/context.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1529549,1530213

Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=1530216&r1=1530215&r2=1530216&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Tue Oct  8 10:34:19 2013
@@ -2223,6 +2223,7 @@ public class WebappClassLoader
     @SuppressWarnings("deprecation") // thread.stop()
     private void clearReferencesThreads() {
         Thread[] threads = getThreads();
+        List<Thread> executorThreadsToStop = new ArrayList<Thread>();
 
         // Iterate over the set of threads
         for (Thread thread : threads) {
@@ -2281,6 +2282,7 @@ public class WebappClassLoader
 
                     // If the thread has been started via an executor, try
                     // shutting down the executor
+                    boolean usingExecutor = false;
                     try {
 
                         // Runnable wrapped by Thread
@@ -2313,6 +2315,7 @@ public class WebappClassLoader
                             Object executor = executorField.get(target);
                             if (executor instanceof ThreadPoolExecutor) {
                                 ((ThreadPoolExecutor) executor).shutdownNow();
+                                usingExecutor = true;
                             }
                         }
                     } catch (SecurityException e) {
@@ -2333,14 +2336,46 @@ public class WebappClassLoader
                                 thread.getName(), contextName), e);
                     }
 
-                    // This method is deprecated and for good reason. This is
-                    // very risky code but is the only option at this point.
-                    // A *very* good reason for apps to do this clean-up
-                    // themselves.
-                    thread.stop();
+                    if (usingExecutor) {
+                        // Executor may take a short time to stop all the
+                        // threads. Make a note of threads that should be
+                        // stopped and check them at the end of the method.
+                        executorThreadsToStop.add(thread);
+                    } else {
+                        // This method is deprecated and for good reason. This
+                        // is very risky code but is the only option at this
+                        // point. A *very* good reason for apps to do this
+                        // clean-up themselves.
+                        thread.stop();
+                    }
                 }
             }
         }
+
+        // If thread stopping is enabled, executor threads should have been
+        // stopped above when the executor was shut down but that depends on the
+        // thread correctly handling the interrupt. Give all the executor
+        // threads a few seconds shutdown and if they are still running
+        // Give threads up to 2 seconds to shutdown
+        int count = 0;
+        for (Thread t : executorThreadsToStop) {
+            while (t.isAlive() && count < 100) {
+                try {
+                    Thread.sleep(20);
+                } catch (InterruptedException e) {
+                    // Quit the while loop
+                    break;
+                }
+                count++;
+            }
+            if (t.isAlive()) {
+                // This method is deprecated and for good reason. This is
+                // very risky code but is the only option at this point.
+                // A *very* good reason for apps to do this clean-up
+                // themselves.
+                t.stop();
+            }
+        }
     }
 
 

Modified: tomcat/tc7.0.x/trunk/webapps/docs/config/context.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/config/context.xml?rev=1530216&r1=1530215&r2=1530216&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/config/context.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/config/context.xml Tue Oct  8 10:34:19 2013
@@ -714,7 +714,10 @@
         is likely to result in instability. As such, enabling this should be
         viewed as an option of last resort in a development environment and is
         not recommended in a production environment. If not specified, the
-        default value of <code>false</code> will be used.</p>
+        default value of <code>false</code> will be used. If this feature is
+        enabled, web applications may take up to two seconds longer to stop as
+        executor threads are given up to two seconds to stop gracefully before
+        <code>Thread.stop()</code> is called on any remaining threads.</p>
       </attribute>
 
       <attribute name="clearReferencesStopTimerThreads" required = "false">



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