Skip to content

Conversation

ppkarwasz
Copy link
Contributor

As showed in #3252, Spring's JndiPropertySource not only can throw exceptions, but can also perform logging calls. Such a call causes a recursive call to PropertiesUtil.getProperty("log4j2.flowMessageFactory") and a StackOverflowException in the best scenario. The worst scenario includes a deadlock.

This PR:

  • Moves the creation of the default MessageFactory and FlowMessageFactory to the static initializer of LoggerContext. This should be close enough to the pre-2.23.0 location in AbstractLogger. The LoggerContext class is usually initialized, before Spring Boot adds its property sources to PropertiesUtil.
  • Adds a check to PropertiesUtil to ignore recursive calls.

Closes #3252.

As showed in #3252, Spring's `JndiPropertySource` not only can throw exceptions, but can also perform logging calls.
Such a call causes a recursive call to `PropertiesUtil.getProperty("log4j2.flowMessageFactory"`) and a `StackOverflowException` in the best scenario. The worst scenario includes a deadlock.

This PR:

- Moves the creation of the default `MessageFactory` and `FlowMessageFactory` to the static initializer of `LoggerContext`. This should be close enough to the pre-2.23.0 location in `AbstractLogger`. The `LoggerContext` class is usually initialized, before Spring Boot adds its property sources to `PropertiesUtil`.
- Adds a check to `PropertiesUtil` to ignore recursive calls.

Closes #3252.
@ppkarwasz ppkarwasz self-assigned this Dec 4, 2024
@ppkarwasz ppkarwasz requested a review from vy December 6, 2024 16:55
The constructor is effectively package-private, since it has a package-private class (`AsyncLoggerDisruptor`) in its signature.
@ppkarwasz ppkarwasz merged commit 18a1deb into 2.24.x Dec 9, 2024
7 checks passed
@ppkarwasz ppkarwasz deleted the fix/2.24.x/3252_recursive_property_source branch December 9, 2024 08:01
@sundaybluesky
Copy link

sundaybluesky commented Mar 5, 2025

Hi @ppkarwasz,

Can you please consider change back AsyncLogger constructor package-private access back to public? I've extended AsyncLogger class with the same package name to build a customized AsyncLogger and use reflection to initial customized AsyncLogger. This code was working for more than 7 years and blocked in 2.24.3 with this PR code change.

Thanks,
sundaybluesky

@ppkarwasz
Copy link
Contributor Author

Hi @sundaybluesky,

Can you please consider change back AsyncLogger constructor package-private access back to public?

Sure, please open a new feature request. In the new feature request, can you explain how are you using AsyncLogger. It is entirely possible that there are already public APIs that allow you to do what you are doing. For example AsyncLoggerContext.newLogger() allows you to create a new AsyncLogger without any reflection.

This code was working for more than 7 years and blocked in 2.24.3 with this PR code change.

That is the problem with using reflection to access private methods or classes like AsyncLoggerDisruptor: the class or method can disappear without notice.

Note: As far as I understand you called the constructor of AsyncLogger through reflection. You can still do it, but you need to make it accessible first.

@sundaybluesky
Copy link

sundaybluesky commented Mar 11, 2025

Hi @ppkarwasz ,

Thanks for you quick response. I created #3527 to track this issue, please help to take a look.

Regards,
sundaybluesky

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants