Skip to content

fix: DH-22426: Disable logback's auto-closing behavior wrt servlets#7958

Open
devinrsmith wants to merge 2 commits intodeephaven:mainfrom
devinrsmith:nightly/dh-22426-remove-auto-logback-stop
Open

fix: DH-22426: Disable logback's auto-closing behavior wrt servlets#7958
devinrsmith wants to merge 2 commits intodeephaven:mainfrom
devinrsmith:nightly/dh-22426-remove-auto-logback-stop

Conversation

@devinrsmith
Copy link
Copy Markdown
Member

Logback registers an implementation of the service jakarta.servlet.ServletContainerInitializer which allows logback to automatically stop when a servlet is stopped. This causes us to lose logging at the end of our shutdown cycle. This PR disables this logic.

See https://logback.qos.ch/manual/configuration.html#webShutdownHook

…are installed

Logback registers an implementation of the service `jakarta.servlet.ServletContainerInitializer` which allows logback to automatically stop when a servlet is stopped. This causes us to lose logging at the end of our shutdown cycle. This PR disables this logic.

See https://logback.qos.ch/manual/configuration.html#webShutdownHook
@devinrsmith devinrsmith self-assigned this Apr 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

No docs changes detected for c4816f7

@devinrsmith
Copy link
Copy Markdown
Member Author

Before:

2026-04-29T01:26:39.577Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Initiating shutdown processing
2026-04-29T01:26:39.578Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Starting to invoke FIRST shutdown tasks
2026-04-29T01:26:39.581Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Done invoking FIRST shutdown tasks
2026-04-29T01:26:39.581Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Starting to invoke MIDDLE shutdown tasks
2026-04-29T01:26:39.586Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Done invoking MIDDLE shutdown tasks
2026-04-29T01:26:39.586Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Starting to invoke LAST shutdown tasks
2026-04-29T01:26:39.587Z | Thread-14            |  INFO | o.e.jetty.server.Server   | Stopped oejs.Server@7c88d04f{STOPPING}[12.0.33,sto=10000]
2026-04-29T01:26:39.587Z | Thread-14            |  INFO | o.e.jetty.server.Server   | Shutdown oejs.Server@7c88d04f{STOPPING}[12.0.33,sto=10000]
2026-04-29T01:26:39.591Z | Thread-14            |  INFO | o.e.j.s.AbstractConnector | Stopped ServerConnector@31c0c7e5{SSL, (ssl, alpn, h2)}{0.0.0.0:8443}

After:

2026-04-29T01:25:07.672Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Initiating shutdown processing
2026-04-29T01:25:07.673Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Starting to invoke FIRST shutdown tasks
2026-04-29T01:25:07.676Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Done invoking FIRST shutdown tasks
2026-04-29T01:25:07.677Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Starting to invoke MIDDLE shutdown tasks
2026-04-29T01:25:07.681Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Done invoking MIDDLE shutdown tasks
2026-04-29T01:25:07.681Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Starting to invoke LAST shutdown tasks
2026-04-29T01:25:07.682Z | Thread-14            |  INFO | o.e.jetty.server.Server   | Stopped oejs.Server@560d6d2{STOPPING}[12.0.33,sto=10000]
2026-04-29T01:25:07.682Z | Thread-14            |  INFO | o.e.jetty.server.Server   | Shutdown oejs.Server@560d6d2{STOPPING}[12.0.33,sto=10000]
2026-04-29T01:25:07.686Z | Thread-14            |  INFO | o.e.j.s.AbstractConnector | Stopped ServerConnector@333a2df2{SSL, (ssl, alpn, h2)}{0.0.0.0:8443}
2026-04-29T01:25:07.690Z | Thread-14            |  INFO | e.s.ServletContextHandler | Stopped oeje10w.WebAppContext@69d902f9{ROOT,/,b=[jar:file:///home/devin/dev/deephaven/deephaven-core/se
rver/jetty-app/build/install/server-jetty/lib/deephaven-web-42.0-SNAPSHOT.jar!/META-INF/resources/, jar:file:///home/devin/.cache/deephaven/.JsPluginsZipFilesystem361410098764990646/deephav
en-js-plugins.zip!/],a=AVAILABLE,h=oeje10ss.ConstraintSecurityHandler@18dafd3b{STOPPED}}
2026-04-29T01:25:07.692Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Done invoking LAST shutdown tasks
2026-04-29T01:25:07.692Z | Thread-0             |  WARN | d.u.p.ShutdownManagerImpl | Finished shutdown processing

@devinrsmith devinrsmith marked this pull request as ready for review April 29, 2026 14:32
@devinrsmith devinrsmith requested a review from niloc132 April 29, 2026 14:34
@devinrsmith
Copy link
Copy Markdown
Member Author

Green nightlies.

// have a logback dependency at this level, our applications that depend on this project typically do. It
// would probably be "more correct" to have a way for the application main's to set or pass along this property
// themselves, but there is little harm in setting it here unconditionally (if the slf4j implementation is not
// logback, this setting will have no effect).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about server/jetty-11/src/main/java/io/deephaven/server/jetty11/JettyBackedGrpcServer.java

Thread newShutdownHookThread() {
return new Thread(() -> {
final boolean didInvokeTasks = maybeInvokeTasks();
// If some other thread has already invoked maybeInvokeTasks, we still want to block shutdown on completion
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want the second thread to wait and not just quit if it sin't actually doing any work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants