Thread stopping is too aggressive

[rhuitl]

I think the thread stopping is a bit too aggressive because it doesn't wait before interrupting threads that are shutting down on their own. It seems like the waiting time is only applied after interrupting running threads, which in the case of Quartz triggers an "ERROR" message.

2015-09-20 18:00:06.699:INFO:/test:Scanner-0: org.tuckey.web.filters.urlrewrite.UrlRewriteFilter INFO: destroy called
[INFO] 2015-09-20 18:00:06,701 org.example.services - Deregistering JDBC drivers
[INFO] 2015-09-20 18:00:06,702 org.example.services - Deregistering JDBC driver org.postgresql.Driver@3cd16578
[INFO] 2015-09-20 18:00:06,702 org.quartz.core.QuartzScheduler - Scheduler MyScheduler_$_NON_CLUSTERED shutting down.
[INFO] 2015-09-20 18:00:06,702 org.quartz.core.QuartzScheduler - Scheduler MyScheduler_$_NON_CLUSTERED paused.
[INFO] 2015-09-20 18:00:06,703 org.quartz.core.QuartzScheduler - Scheduler MyScheduler_$_NON_CLUSTERED shutdown complete.
[INFO] 2015-09-20 18:00:06,708 org.quartz.ee.servlet.QuartzInitializerListener - Quartz Scheduler successful shutdown.
ClassLoaderLeakPreventor: se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventor shutting down context by removing known leaks (CL: 0x60d6d6d4)
ClassLoaderLeakPreventor: Internal registry of java.beans.PropertyEditorManager not found
ClassLoaderLeakPreventor: Looping 0 RMI Targets to find leaks
ClassLoaderLeakPreventor: Looping 0 RMI Targets to find leaks
ClassLoaderLeakPreventor: Stopping Thread 'Thread[MyScheduler_Worker-1,5,main]' of type org.quartz.simpl.SimpleThreadPool$WorkerThread running in web app after 5000 ms 
[ERROR] 2015-09-20 18:00:06,742 org.quartz.simpl.SimpleThreadPool - Worker thread was interrupt()'ed.
java.lang.InterruptedException
        at java.lang.Object.wait(Native Method)
        at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:568)
ClassLoaderLeakPreventor: Since Java 1.6+ is used, we can call public static final void java.util.ResourceBundle.clearCache(java.lang.ClassLoader)
ClassLoaderLeakPreventor: Releasing web app classloader from Apache Commons Logging
2015-09-20 18:00:06.746:INFO:oejsh.ContextHandler:Scanner-0: Stopped o.e.j.w.WebAppContext@59e0b2fa{/test,file:/tmp/jetty-0.0.0.0-8080-test.war-_test-any-2539080646960400183.dir/webapp/,UNAVAILABLE}{/tmp/test.war}

Looking at the source of SimpleThreadPool.java I see that there's a shutdown mechanism for the worker thread. Without Classloader Leak Prevention, the worker thread shuts down cleanly.

Wouldn't it be better to wait for some seconds, then interrupt running threads, and finally wait another few seconds for them to finish before calling stop()?

[mjiderhamn]

Hi rhuitl, and thank you for taking the time to report. However, I tend to disagree with you on this. interrupt() only affects threads that are waiting, where InterruptedException is a checked exception and must be handled.

In the Quartz SimpleThreadPool.WorkerThread case, the thread is waiting for a new job to run. If it is interrupt()ed vs shut down, it will run though the same code path - with the logging of the InterruptedException being the only difference. Meaning, you can safely ignore the error. I do agree it would have been prettier without the error, but (at this point) I don't think this is reason enough to change the behaviour of the ClassLoader Leak Preventor, because in the generic case that means shutdown will take more time.

I could imagine adding a setting for the wait time before calling interrupt, and by default have that wait disabled. You are welcome to contribute a pull request if that seems like a good idea!

[rhuitl]

Hi Mattias,

I understand that you don't want to add an extra delay in the generic case. I'll try to come up with a patch that will not introduce extra delay if threads shutdown cleanly on their own. Only when they don't, there should be a configurable waiting time before interrupt() is called. Anyhow, that should not be the generic case because I assume that people will fix their webapp if they see that ClassLoader Leak Preventor has to kill threads.

Robert