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

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

Closed
centic9 opened this issue Nov 10, 2024 · 17 comments
Closed
Labels
api Affects the public API bug Incorrect, unexpected, or unintended behavior of existing code waiting-for-maintainer

Comments

@centic9
Copy link
Member

centic9 commented Nov 10, 2024

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
Copy link
Member

Hello @centic9

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

@centic9
Copy link
Member Author

centic9 commented Nov 10, 2024

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
Copy link
Member

vy commented Nov 11, 2024

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

@vy vy added bug Incorrect, unexpected, or unintended behavior of existing code api Affects the public API waiting-for-user More information is needed from the user labels Nov 11, 2024
@centic9
Copy link
Member Author

centic9 commented Nov 11, 2024

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.

@vy vy added waiting-for-maintainer and removed waiting-for-user More information is needed from the user labels Nov 11, 2024
@ppkarwasz
Copy link
Contributor

@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
Copy link

xzel23 commented Nov 14, 2024

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
Copy link
Contributor

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:

  • Log4j API 2.24.x is backward compatible with Log4j API 2.23.x, so you can use library A, B and log4j-api 2.24.x. Since we use semantic versioning (and we use BND Baseline to check API compatibility) if you have an application with 42 libraries that use Log4j API, you just need to pick the most recent version.
    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.
  • Even if library A declares a dependency on Log4j API 2.24.x, it will probably work with Log4j API 2.13.x or so. Log4j API and Log4j Core have a common versioning scheme. Most of the time the minor version bumps are due to Log4j Core. The last methods added to the public Log4j API were around 2.13.x.

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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link

xzel23 commented Nov 15, 2024

@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.

centic9 added a commit to centic9/oss-fuzz that referenced this issue Nov 17, 2024
Apache POI downgraded log4j-api due to issue
apache/logging-log4j2#3196

The upcoming log4j-(api|core) 2.24.2 hopefully will be
forward/backwards compatible again
@ppkarwasz
Copy link
Contributor

@centic9,

Does version 2.24.2 (RC1) solve your problem?

@centic9
Copy link
Member Author

centic9 commented Nov 19, 2024

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

@ed-erwin-tf
Copy link

I was running into this problem with using poi. I was declaring a specific version of log4j-core (2.24.0 or 2.24.1), not realizing that poi was declaring an older version of log4j-api (2.23.1). I can solve my problem by explicitly declaring 2.24.1 for both core and api. But it took quite a while to track this down.

@ppkarwasz
Copy link
Contributor

Closing this, since release 2.24.2 fixes the NPE.

@ppkarwasz
Copy link
Contributor

@ed-erwin-tf,

Please, use our log4j-bom BOM to manage dependencies, see our Installation guide for more details

Maven's conflict resolution algorithm can not possible know that between apache-poi and log4j-core, the latter should dictate the version of the Log4j API.

@xzel23
Copy link

xzel23 commented Nov 22, 2024

@ppkarwasz while the release page mentions a release date of November 18, 2.24.2 is still not available on Maven central.

@ppkarwasz
Copy link
Contributor

@xzel23,

We generate the date in the release notes using our internal log4j-changelog-maven-plugin, so the release date is actually the date we generated the artifacts, not the date the release vote ended.

I have created apache/logging-log4j-tools#158 to make the default release date value more Apache-like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the public API bug Incorrect, unexpected, or unintended behavior of existing code waiting-for-maintainer
Projects
None yet
Development

No branches or pull requests

7 participants