Skip to content

Fix/missing constructor - #4433

Merged
ppkarwasz merged 6 commits into
mainfrom
fix/missing-constructor
Oct 28, 2026
Merged

ppkarwasz merged 6 commits into
mainfrom
fix/missing-constructor

Conversation

@rgoers

@rgoers rgoers commented Oct 26, 2026

Copy link
Copy Markdown
Member

Add back a constructor that was removed that Spring Boot uses.

Fix LoggerUpdaterTest that was broken after a different PR was applied. Claude performed the fixes:
Changed two tests in LoggerUpdateTest.java:

  • testUpdateLoggersPropertyListeners: it now keeps a reference to its configuration-started listener and removes it in a finally block.
  • propertyChangeListenerReceivesOldAndNewConfigurationOnReconfiguration: in a finally block, it now removes its property-change listener and calls context.reconfigure().

@rgoers
rgoers requested review from jvz and ppkarwasz October 26, 2026 17:14
@rgoers
rgoers changed the base branch from 2.x to main October 26, 2026 17:16
Comment thread log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java Outdated
@ramanathan1504

Copy link
Copy Markdown
Contributor

The LoggerUpdateTest change is needed: on main the leaked configuration-started listener fails propertyChangeListenerReceivesOldAndNewConfigurationOnReconfiguration, and with the finally blocks the class passes.
The restored LoggerConfig(String, Level, boolean) passes a null configuration to the protected constructor, which calls config.getLogEventFactory(), so Spring Boot's LevelSetLoggerConfig now gets a NullPointerException instead of NoSuchMethodError. Where should it get its LogEventFactory and ReliabilityStrategy without a configuration, and can a test build one, add it with addLogger and log through it?

@ramanathan1504

Copy link
Copy Markdown
Contributor

@rgoers Changelog also needed.
Thanks

@ramanathan1504 ramanathan1504 added the port-done No further port needed: all required ports are merged, or none was needed label Oct 27, 2026
Comment thread src/changelog/.3.x.x/4433_logger_config.xml
@ramanathan1504

Copy link
Copy Markdown
Contributor

The NPE is gone: a LoggerConfig from this constructor gets the context's configuration and logs after addLogger and updateLoggers.
Can LoggerUpdateTest get a test that does exactly that, with removeLogger in a finally? None of the current tests call this constructor, so a revert to null would stay green.

@ppkarwasz
ppkarwasz merged commit 41d8320 into main Oct 28, 2026
10 checks passed
@ppkarwasz
ppkarwasz deleted the fix/missing-constructor branch October 28, 2026 16:49
@github-project-automation github-project-automation Bot moved this from Approved to Merged in Log4j pull request tracker Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

port-done No further port needed: all required ports are merged, or none was needed

Projects

Development

Successfully merging this pull request may close these issues.

4 participants