search

apache / logging-log4j2

Apache Log4j 2 is a versatile, feature-rich, efficient logging API and backend for Java.
https://logging.apache.org/log4j/2.x/
Apache License 2.0
3.4k stars 1.62k forks source link

log4j-api 2.24.1 causes getLogger() to return null when using log4j-core 2.24.0 #3196

Open centic9 opened 1 week ago

centic9 commented 1 week ago

Description

When upgrading log4j-api to 2.24.1 while still using log4j-core 2.24.0, LogManager.getLogger() returns null.

This seems to be caused by changes to LoggerRegistry in log4j-api.

This version-mismatch happens easily if a library like Apache POI upgrades "api" to the latest version, but projects using it do not upgrade log4j-core along the way. This is likely a common situation as libraries should only depend on "api" and thus cannot enforce a newer version of "core".

It's unexpected to run into such an incompatibility, especially in a minor bugfix-release of log4j.

Configuration

Version: api 2.24.1, core 2.24.0

Operating system: Linux

JDK: JDK 11 + 17

Logs

Code is simply

public static final Logger LOG = LogManager.getLogger(Class.class);

This later triggers a NullPointerExceptions as LOG is null.

Reproduction

Run ./gradlew runWriteFile -PpoiVersion=5.4.0 in a checkout of https://github.com/centic9/poi-reproduce

garydgregory commented 1 week ago

Hello @centic9

Is there a reason your app uses a split version? Is this intentional?

centic9 commented 1 week ago

As log4j-api and log4j-core are defined at different places, the exact versions that people end up with will likely often differ.

E.g. I am looking at it as commiter of the Apache POI library. We upgraded log4j-api to 2.24.1 and plan to roll a release.

As Apache POI is just a library, it does not contain a dependency on log4j-core. Users of the library usually add log4j-core or another implementation of the api.

When users of Apache POI upgrade without adjusting log4j-core along the way, they will run into this issue.

Yes, ideally log4j-api and log4j-core would be the same version always, but I expect quite some bug-reports if we release Apache POI this way as reality will see version-differences fairly frequently.

vy commented 1 week ago

@centic9, #2936 is probably fixed your problem. Could you check if you can still reproduce the problem using 2.25.0-SNAPSHOT, please?

centic9 commented 1 week ago

I tried with log4j-api 2.25.0-SNAPSHOT as-of Nov. 9th and log4j-cor 2.24.0, this still showed the same, so I don't think #2936 fixes this.

ppkarwasz commented 6 days ago

@centic9,

E.g. I am looking at it as commiter of the Apache POI library. We upgraded log4j-api to 2.24.1 and plan to roll a release.

As Apache POI is just a library, it does not contain a dependency on log4j-core. Users of the library usually add log4j-core or another implementation of the api.

When users of Apache POI upgrade without adjusting log4j-core along the way, they will run into this issue.

Yes, ideally log4j-api and log4j-core would be the same version always, but I expect quite some bug-reports if we release Apache POI this way as reality will see version-differences fairly frequently.

I believe that your problem is deeper than the current bug in Log4j API 2.24.1.

In general the minor version of log4j-core must be equal or higher than the minor version of log4j-api. The API might introduce new methods in a minor update, that the older Log4j Core does not implement. This is a natural requirement, so compatibility issues between log4j-api version 2.25.0 and log4j-core version 2.24.1 are to be expected and are not bugs! On the other hand log4j-api is (unfortunately IMHO) not a pure API and contains helper methods that log4j-core uses. That gives a de facto requirement for the minor version of log4j-api to be equal or higher than the minor version of log4j-core. This is probably a bug!

Normally, to solve the problem you are having in Apache POI, I would recommend to stick to an older version of log4j-api (e.g. 2.18.0). This is however only a theoretical recommendation, in practice we made a lot of changes in Log4j 2.24.0 to allow Log4j API to work with log4j-core version 3.x. The side effect of these changes is that log4j-core version 2.24.x is incompatible with all previous releases of log4j-api.

xzel23 commented 5 days ago

Now that's really bad. How are library authors and application developers supposed to cope with this? Do I understand correctly that if I have a dependency on lib A that is on log4j-api 2.23 and a lib B that is on 2.24, there's nothing I can do?

That would make the whole point of using log4j-api at all rather useless. Whatever features newer versions add, it should be possible to use a lower minor version of log4j-api together with a newer log4j-core. Everything that breaks that does not belong in log4j-api.

If it is as really you say, using log4j-api cannot be recommended to any library developer. Fixing this should have the highest priority in the log4j project and you should add test that make sure log4j-core works as expected with whatever version of log4j-api of the same major release is used.

Given the current status, this might not be possible for already released versions 2.x, but at least make sure to have a 2.25 release of log4j-api in the near future that will be usable with every log4j-core 2.25+. If you really need to make binary breaking changes in log4j-api, these should go into 3.x.

ppkarwasz commented 5 days ago

Now that's really bad. How are library authors and application developers supposed to cope with this? Do I understand correctly that if I have a dependency on lib A that is on log4j-api 2.23 and a lib B that is on 2.24, there's nothing I can do?

In practice the problem is not as bad as it seems:

The nature of the problem is different: once you chose the version of Log4j API, you should use Log4j Core of the same version. The log4j-bom artifact can ensure this.

Note that this version alignment problem is common to all Java libraries split into API and implementation. If you write an application using Servlet API 2.0 and you deploy it on a Servlet API 2.4 server, at runtime Servlet API 2.0 will not be used, the server will provide Servlet API 2.4 to your application!

If it is as really you say, using log4j-api cannot be recommended to any library developer.

SLF4J suffers from the same problems, without the additional help of semantic versioning. For example SLF4J 2.0.10 introduced a new Reporter helper class. Every Logback version that uses this class is no longer compatible with SLF4J 2.0.9!

Note that some incompatibility problems with previous version of the API are unavoidable: the introduction of a new LogBuilder interface and Logger.atLevel() method in Log4j API 2.13.0 is not a breaking change, but causes Log4j Core 2.13.0 to throw NoClassDefFound errors if Log4j API 2.12.0 is present on the classpath.

Fixing this should have the highest priority in the log4j project and you should add test that make sure log4j-core works as expected with whatever version of log4j-api of the same major release is used.

I agree that we should test our Log4j Core releases against older releases of Log4j API. As I explained above log4j-core version 2.0.1 can not possibly work with log4j-api version 2.24.1, because it lacks the implementation of all the abstract methods that were introduced in the meantime. Such a combination will cause a lot of AbstractMethodErrors. On the other hand we could try to develop Log4j API more carefully to allow Log4j Core 2.25.0 to be compatible with Log4j API 2.24.0. As I stated above, this is a stronger requirement that no breaking changes.

I started a thread on dev@logging, feel free to explain your point of view.

pjfanning commented 5 days ago

@ppkarwasz returning null from an API that users have long expected to not support null returns (LogManager.getLogger) - that is a really big jolt. Could you consider adding support for having LogManager.getLogger return a no-op logger if there are issues creating one? We could log to system.err if we are forced to failover to a no-op logger instance.

ppkarwasz commented 5 days ago

@pjfanning,

@ppkarwasz returning null from an API that users have long expected to not support null returns (LogManager.getLogger) - that is a really big jolt.

Yes, it is a big oversight, but it has nothing to do with version misalignment between log4j-api and log4j-core. The bug appears if there is a GC-run between to consecutive method call. There is #3199 ready to fix it. Feel free to review it.

xzel23 commented 4 days ago

@ppkarwasz Thank you for your reply. So I think it's really not as bad as it seemed at first.

Unfortunately this is not what Maven's conflict resolution does, that's why we encourage people to use log4j-bom for dependency management. It is the first step in our installation guide.

And that's one thing I like about Gradle, because they use another resolution strategy that I think works better in most cases.

SLF4J suffers from the same problems, without the additional help of semantic versioning. For example SLF4J 2.0.10 introduced a new Reporter helper class. Every Logback version that uses this class is no longer compatible with SLF4J 2.0.9!

Of course when you actually use a new feature that wasn't there before, you will probably need an implementation that's at least that version. But if you only upgrade the dependency without further changes to the code, it should not break. I think we all update versions when the used version has a CVE reported. If it's not possible, it should be noted in the release notes and a patch version of the old version should be released.

The bug is annoying, but at least I am relieved to see that you and the team are aware and hopefully we won't see less of such problems.

ppkarwasz commented 1 day ago

@centic9,

Does version 2.24.2 (RC1) solve your problem?

centic9 commented 13 hours ago

I used poi-reproduce to test combinations of 2.23.1, 2.24.0, 2.24.1 and upcoming 2.24.2.

Summary: 2.24.2 seems to solve this issue. Combining 2.23.x and 2.24.2 will still not work, but this is expected/accepted as newer methods in API are needed by newer CORE.

- API: 2.23.1, CORE: 2.23.1: SUCCEED 
- API: 2.23.1, CORE: 2.24.0: FAIL, but accepted/expected
- API: 2.23.1, CORE: 2.24.1: FAIL, but accepted/expected
- API: 2.23.1, CORE: 2.24.2: FAIL, but accepted/expected

- API: 2.24.0, CORE: 2.23.1: FAIL, but accepted/expected
- API: 2.24.0, CORE: 2.24.0: SUCCEED 
- API: 2.24.0, CORE: 2.24.1: SUCCEED 
- API: 2.24.0, CORE: 2.24.2: SUCCEED 

- API: 2.24.1, CORE: 2.23.1: FAIL, but accepted/expected
- API: 2.24.1, CORE: 2.24.0: FAIL, discussed here
- API: 2.24.1, CORE: 2.24.1: SUCCEED 
- API: 2.24.1, CORE: 2.24.2: SUCCEED 

- API: 2.24.2, CORE: 2.23.1: FAIL, but accepted/expected
- API: 2.24.2, CORE: 2.24.0: SUCCEED
- API: 2.24.2, CORE: 2.24.1: SUCCEED 
- API: 2.24.2, CORE: 2.24.2: SUCCEED