[GitHub] [tomcat] pzygielo commented on a change in pull request #275: BZ 59203 - interrupt tomcat threads instead of stopping

2020-04-20 Thread GitBox


pzygielo commented on a change in pull request #275:
URL: https://github.com/apache/tomcat/pull/275#discussion_r410846802



##
File path: java/org/apache/catalina/loader/WebappClassLoaderBase.java
##
@@ -1813,35 +1813,40 @@ private void clearReferencesThreads() {
 // stopped and check them at the end of the method.
 executorThreadsToStop.add(thread);
 } else {
-thread.interrupt();
+clearThread(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
-int count = 0;
+// clear executor threads
 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();
-}
+clearThread(t);
+}
+}
+
+private void clearThread(Thread t) {
+int count = 0;
+if (!t.isInterrupted()) {
+t.interrupt();
+}
+
+// Give threads up to 2 seconds to shutdown
+while (t.isAlive() && count < 100) {
+try {
+Thread.sleep(20);
+} catch (InterruptedException e) {
+// Quit the while loop

Review comment:
   https://rules.sonarsource.com/java/RSPEC-2142





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



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



[GitHub] [tomcat] pzygielo commented on a change in pull request #275: BZ 59203 - interrupt tomcat threads instead of stopping

2020-04-19 Thread GitBox
pzygielo commented on a change in pull request #275: BZ 59203 - interrupt 
tomcat threads instead of stopping
URL: https://github.com/apache/tomcat/pull/275#discussion_r410846802
 
 

 ##
 File path: java/org/apache/catalina/loader/WebappClassLoaderBase.java
 ##
 @@ -1813,35 +1813,40 @@ private void clearReferencesThreads() {
 // stopped and check them at the end of the method.
 executorThreadsToStop.add(thread);
 } else {
-thread.interrupt();
+clearThread(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
-int count = 0;
+// clear executor threads
 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();
-}
+clearThread(t);
+}
+}
+
+private void clearThread(Thread t) {
+int count = 0;
+if (!t.isInterrupted()) {
+t.interrupt();
+}
+
+// Give threads up to 2 seconds to shutdown
+while (t.isAlive() && count < 100) {
+try {
+Thread.sleep(20);
+} catch (InterruptedException e) {
+// Quit the while loop
 
 Review comment:
   https://rules.sonarsource.com/java/RSPEC-2142


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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