> On Aug 31, 2020, at 12:27 PM, Rémy Maucherat <r...@apache.org> wrote: > > > On Mon, Aug 31, 2020 at 8:39 PM Mark Thomas <ma...@apache.org> wrote: > >> On 31/08/2020 17:17, David Blevins wrote: > >> Hey All, > >> > >> Curious if there would be any willingness to change the default of this to > >> true for Tomcat 10 prior to final release. > >> > >> - > >> https://github.com/apache/tomcat/blob/master/java/org/apache/catalina/core/StandardServer.java#L195 > > > > Unlikely based on the information provided. Making the utility threads > > daemon threads looks like a hack to work around a problem elsewhere. I'm > > more interested in tackling the root cause. > > I added the option because "you never know", but I don't quite see why it's a > good idea to interrupt this sort of work.
I think the flag is only useful because the current default is true. The first default you put was false and I definitely agree with your commit message on it: "Those threads are not bad candidates for non daemon by default, given they are managed by an executor." [1] I agree with that as historically all pools in Tomcat were set as daemons, including all http thread pools. The main thread where `Catalina.start()` ran was the only non-daemon thread that could prevent a JVM exit. If you didn't set `await` in Catalina you got the intended consequence that no threads would be left holding the JVM open. This was a great contract for embedded environments like unit tests. In these scenarios where you did not want Tomcat internals hold the JVM open you just needed to take care to set `await` to false and all worked fine. The concern is when we add new threads that don't adhere to that contract everyone's embedded setups now break. - https://stackoverflow.com/questions/55382691/how-to-check-why-jvm-is-not-able-to-terminate - https://github.com/spring-projects/spring-boot/issues/17139 The reason I say the flag is limitedly useful is as Mark points out this flag doesn't have any influence over Tomcat standalone usage. There await is set to true and therefore the explicit `stop()` phase is called which will shutdown the utility pool, daemon or not. So while the change was invisible to standalone users, for all embedded users it was a breaking change over 9.0.13. When we introduce a breaking change like that causes people's unit tests or embedded environments to not terminate, everyone has to learn the hard way about the new flag and figure out how to work it in. I think the root issue is not actually about which is safer daemon or non-daemon threads, really more about adding new shutdown requirements in the middle of a major version. All the refactoring you did that whole month does underline how even with a lot of deliberation it's very tough to know which way to lean. It's usually those flip-flop situations where it's not clear what the right answer is that end up getting you. I'm incredibly grateful there is at least a flag there. What I would recommend is we agree on some sort of daemon thread policy. Some options: - All threads outside Catalina's "main" should be daemon by default - The daemon setting of threads outside Catalina's "main" should default to the value of Catalina.await - The daemon setting of threads outside Catalina's "main" should default to the value of some new very well-documented setting Thoughts? -David [1] https://github.com/apache/tomcat/commit/75489c324730511ab5ff12a0ee3606b7c8926726
smime.p7s
Description: S/MIME cryptographic signature
