Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slf4JBridgeHandler.uninstall called too early in context close, swallowing JUL log output. #33697

Open
sijskes opened this issue Jan 5, 2023 · 6 comments
Labels
type: bug A general bug
Milestone

Comments

@sijskes
Copy link

sijskes commented Jan 5, 2023

Behavior

JUL logging output in @PreDestroy methods is swallowed, not logged.

Expected behavior

JUL logging output in @PreDestroy methods logged.

Description

LogbackLoggingSystem.cleanup() -> Slf4JLoggingSystem.cleanup() -> Slf4JBridgeHandler.uninstall()

is called before @PreDestroy methods.

therefore JUL logging in @PreDestroy is swallowed, not logged.

This is because the uninstall was registered as a SmartLifeCycle, in LoggingApplicationListener.onApplicationPreparedEvent.

And SmartLifeCycle.stop() is called early in the AbstractApplicationContext.doClose(), before the destroyBeans().

Versions

  • spring-boot-2.6.13.jar / spring-boot-2.7.7.jar
  • jul-to-slf4j-1.7.36.jar

Context

  • WAR deployment
  • Apache Tomcat Version 9.0.53

Misc

There are multiple call paths reaching LoggingApplicationListener.cleanupLoggingSystem(). Also one for onContextClosedEvent(). but this contains shortcut code for exactly this scenario. So it looks like the SmartLifeCycle shutdown route is intentional, but unclear to me why the shutdown of Slf4JBridgeHandler should happen so early in the shutdown.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 5, 2023
@wilkinsona
Copy link
Member

This behaviour was improved in 45ad155 but it sounds like it didn't go far enough for your needs. I'm not sure if we'll be able to improve things further, but we can take a look. To help us to do so, please provide a complete yet minimal sample that reproduces the problem. You can share such a sample with us by pushing it to a separate repository on GitHub or by zipping it up and attaching it to this issue.

Please note that Spring Boot 2.6.x is no longer eligible for open source support. Any fix for this issue will only be made in 2.7.x at the earliest.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Jan 5, 2023
@sijskes
Copy link
Author

sijskes commented Jan 5, 2023

When it turns out that it might not be easy to fix this in AbstractApplicationContext.doClose(), we could discuss the need of calling the Slf4JLoggingSystem.removeJdkLoggingBridgeHandler() in Slf4JLoggingSystem.cleanup(). All it does is removing a reference from the JUL logging handlers lists, no listener calling or cleaning resources (so it looks).

It would be easily fixed by adding a property which shortcuts this behaviour, but i'm not familiar with how easy this is perceived in the whole spring project.

I will try to bump the version to a higher level, this will take some time. If this doesnt fix things, i will build minimal sample.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 5, 2023
@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 5, 2023
@sijskes
Copy link
Author

sijskes commented Jan 5, 2023

upgraded tot spring boot 2.7.6.

what stands out here, that although Slf4JLoggingSystem decides not to install SLF4JBridgeHandler in configureJdkLoggingBridgeHandler() it still decides to uninstall it. That to me seems asymmetric.

So any change of handling SLF4JBridgeHandler in the class derived from SpringBootServletInitializer is impossible.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 5, 2023
@sijskes
Copy link
Author

sijskes commented Jan 6, 2023

logbugdemo.zip

minimal example demonstrating this bug

@sijskes
Copy link
Author

sijskes commented Jan 6, 2023

stackdump of preDestroy();

Screenshot at 2023-01-05 21-55-46

stackdump of cleanup();

Screenshot at 2023-01-05 22-00-40

proposed changes:

  • do not shutdown logging subsystem, that you did not start yourself. (re: Slf4JLoggingSystem)
  • shutdown logging at the latest moment (re: AbstractApplicationContext)

The first one is the most fundamental one, if applied it allows you to control the logging bridge at application level.

@sijskes
Copy link
Author

sijskes commented Jan 6, 2023

when deployed with tomcat the Slf4JLoggingSystem.isJulUsingASingleConsoleHandlerAtMost() returns false dependent on the logging configuration used in the tomcat server.

@wilkinsona wilkinsona removed the status: feedback-provided Feedback has been provided label Jan 6, 2023
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 4, 2024
@wilkinsona wilkinsona added this to the 3.2.x milestone Jun 4, 2024
@philwebb philwebb modified the milestones: 3.2.x, 3.3.x Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants