tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject [tomcat] branch 7.0.x updated: Fix BZ 59203 - Try Thread.interrupt() before Thread.stop()
Date Thu, 23 Apr 2020 11:58:31 GMT
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/7.0.x by this push:
     new c8ef0bd  Fix BZ 59203 - Try Thread.interrupt() before Thread.stop()
c8ef0bd is described below

commit c8ef0bd6ac1da711dc253d1e2306528741c01639
Author: Mark Thomas <markt@apache.org>
AuthorDate: Thu Apr 23 12:29:18 2020 +0100

    Fix BZ 59203 - Try Thread.interrupt() before Thread.stop()
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=59203
---
 .../catalina/loader/WebappClassLoaderBase.java     | 36 +++++++++++-----------
 webapps/docs/changelog.xml                         |  7 +++++
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/java/org/apache/catalina/loader/WebappClassLoaderBase.java b/java/org/apache/catalina/loader/WebappClassLoaderBase.java
index 91f2a83..968c215 100644
--- a/java/org/apache/catalina/loader/WebappClassLoaderBase.java
+++ b/java/org/apache/catalina/loader/WebappClassLoaderBase.java
@@ -2517,7 +2517,7 @@ public abstract class WebappClassLoaderBase extends URLClassLoader
     @SuppressWarnings("deprecation") // thread.stop()
     private void clearReferencesThreads() {
         Thread[] threads = getThreads();
-        List<Thread> executorThreadsToStop = new ArrayList<Thread>();
+        List<Thread> threadsToStop = new ArrayList<Thread>();
 
         // Iterate over the set of threads
         for (Thread thread : threads) {
@@ -2626,29 +2626,29 @@ public abstract class WebappClassLoaderBase extends URLClassLoader
                                 thread.getName(), contextName), e);
                     }
 
-                    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();
+                    // Stopping an executor automatically interrupts the
+                    // associated threads. For non-executor threads, interrupt
+                    // them here.
+                    if (!usingExecutor && !thread.isInterrupted()) {
+                        thread.interrupt();
                     }
+
+                    // Threads are expected to take a short time to stop after
+                    // being interrupted. Make a note of all threads that are
+                    // expected to stop to enable them to be checked at the end
+                    // of this method.
+                    threadsToStop.add(thread);
                 }
             }
         }
 
-        // 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
+        // If thread stopping is enabled, threads should have been stopped above
+        // when the executor was shut down or the thread was interrupted but
+        // that depends on the thread correctly handling the interrupt. Check
+        // each thread and if any are still running give all threads up to a
+        // total of 2 seconds to shutdown.
         int count = 0;
-        for (Thread t : executorThreadsToStop) {
+        for (Thread t : threadsToStop) {
             while (t.isAlive() && count < 100) {
                 try {
                     Thread.sleep(20);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c49e4ab..a06c14b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -67,6 +67,13 @@
         perform MIME type mapping based on file extension in a case insensitive
         manner. (markt)
       </add>
+      <add>
+        <bug>59203</bug>: Before calling <code>Thread.stop()</code>
(if
+        configured to do so) on a web application created thread that is not
+        stopped by the web application when the web application is stopped, try
+        interrupting the thread first. Based on a pull request by Govinda
+        Sakhare. (markt)
+      </add>
       <fix>
         <bug>64226</bug>: Reset timezone after parsing a date since the date
         format is reused. Test case submitted by Gary Thomas. (remm)


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


Mime
View raw message