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 2020/04/23 11:40:40 UTC
[tomcat] branch master updated: Fix BZ 59203 - Try
Thread.interrupt() before Thread.stop()
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 2e52596 Fix BZ 59203 - Try Thread.interrupt() before Thread.stop()
2e52596 is described below
commit 2e525965521d8c6288b5745113ee5dc906c3be62
Author: Mark Thomas <ma...@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 b9439a2..4d83153 100644
--- a/java/org/apache/catalina/loader/WebappClassLoaderBase.java
+++ b/java/org/apache/catalina/loader/WebappClassLoaderBase.java
@@ -1712,7 +1712,7 @@ public abstract class WebappClassLoaderBase extends URLClassLoader
@SuppressWarnings("deprecation") // thread.stop()
private void clearReferencesThreads() {
Thread[] threads = getThreads();
- List<Thread> executorThreadsToStop = new ArrayList<>();
+ List<Thread> threadsToStop = new ArrayList<>();
// Iterate over the set of threads
for (Thread thread : threads) {
@@ -1808,29 +1808,29 @@ public abstract class WebappClassLoaderBase extends URLClassLoader
thread.getName(), getContextName()), 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 94faf81..3688633 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -52,6 +52,13 @@
<code>AprLifecycleListener</code> so that the only way to use the APR
connectors is to set the full class name. (remm)
</update>
+ <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>62912</bug>: Don't mutate an application provided content header if
it does not contain a charset. Also remove the outdated workaround for
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org