Currently when tomcat is run with eclipse debugger and a Context is reloaded, each thread that exits (because of the feature to renew threads) triggers a breakpoint in eclipse if its "Suspend execution on uncaught exceptions" is enabled (which is the default). This has been reported to eclipse : https://bugs.eclipse.org/bugs/show_bug.cgi?id=384073 which closed it without change in eclipse. This also has a high number of votes on http://stackoverflow.com/questions/6290470/eclipse-debugger-always-blocks-on-threadpoolexecutor-without-any-obvious-excepti and is deemed as quite annoying. The attached patch fixes tomcat by not letting the exception go out of Thread.run() and thus trigger the debugger. Instead, a custom exception is thrown to have the Thread go out of the ThreadPool and then caught and swallowed before Thread.run() returns. Patch is against tomcat 8 trunk and passes checkstyle and NIO2 tests.
Created attachment 31592 [details] proposed patch for tomcat 8
My question would be why there is an exception thrown here to handle this case at all. Instead, why not check to see if we should die cleanly, then emit the recycling message to the log and simply exit the run method? I don't understand why an exception is being used here at all.
(In reply to Christopher Schultz from comment #2) > My question would be why there is an exception thrown here to handle this > case at all. Instead, why not check to see if we should die cleanly, then > emit the recycling message to the log and simply exit the run method? > > I don't understand why an exception is being used here at all. Because actually in normal operation those threads never go out of java.util.concurrent.ThreadPoolExecutor.runWorker(Worker) unless there are more threads than the corePoolSize and the task queue is empty. So, when I worked on the renewal of threads to avoid classloader leaks upon undeployment, the only way I found was to throw an exception in our implementation of ThreadPoolExecutor.afterExecute(Runnable, Throwable). Is it OK for me to commit on tomcat 8 trunk?
Trunk is CTR, so fire away. If you wrote the original code, I can't think of a better person to refactor it :)
committed to tomcat 8 trunk : r1593132 what about tomcat 7 ? is it still CTR as stated in https://wiki.apache.org/tomcat/TomcatVersions
I merged to tomcat 7 : r1593392 The fix will be available in tomcat 7.0.54 and tomcat 8.0.6