search

qos-ch / reload4j

reload4j is a drop-in replacement for log4j 1.2.17
Apache License 2.0
148 stars 22 forks source link

Replacing log4j with reload4j can cause NoSuchFieldError #41

Open scott-kirk opened 2 years ago

scott-kirk commented 2 years ago

This appears to be an interaction between certain versions of the slf4j-log4j12 library and reload4j. If reload4j is used instead of log4j, then using MDC will cause the following stack trace:

Exception in thread "main" java.lang.NoSuchFieldError: tlm
    at org.apache.log4j.MDCFriend.fixForJava9(MDCFriend.java:11)
    at org.slf4j.impl.Log4jMDCAdapter.<clinit>(Log4jMDCAdapter.java:38)
    at Main.main(Main.java:5)

This was discovered by trying to replace log4j with reload4j in the apache Kafka repository, https://github.com/apache/kafka/blob/6eed7743ff6c0e73d65c09bac2e2ad9586cc56ce/gradle/dependencies.gradle#L174

It only occurs in Java versions above 8. The minimal reproduction can be found here: https://github.com/scott-kirk/reload4j-exception I know it's slf4j code that's directly causing this, and it is fixed by upgrading slf4j to the latest version, I was just caught off guard by this issue and think it'd be useful to either explicitly say to use the latest version of slf4j or to somehow apply a workaround to avoid needing to also upgrade slf4j.

ceki commented 2 years ago

Thank you for this detailed report.

The following analysis is inaccurate and should not be heeded. It is preserved here for the historical record.

org.apache.log4j.MDCFriend shipping with slf4j-log4j12 tries to repair log4j1.x MDC on the fly. This reparation fails in modularized applications and in JDK versions 11(?) and later. The same identical exception is thrown if slf4j-log4j and log4j 1.2.17 are on the classpath.

Thus, the issue is not caused by relaod4j but by slf4j-log4j12 trying to fix log4j's MDC and failing to do so due to new JDK reflection restrictions.

scott-kirk commented 2 years ago

Thanks for the response and the explanation. The link you provided is useful, but could you elaborate on The same identical exception is thrown if slf4j-log4j and log4j 1.2.17 are on the classpath? In my repo with the reproduction if I edit the gradle file and comment out reload4j and uncomment log4j there's no error thrown so I don't seem to have the behavior described. It's an easy enough fix to just upgrade slf4j but I suppose I'm just wondering why the docs say it has more to do with the jdk version when in my repo there's no error when using log4j.

This could be just an interaction with the specific versions I'm using, but I just want to make sure we're on the same page

ceki commented 2 years ago

Thanks for the response and the explanation. The link you provided is useful, but could you elaborate on The same identical exception is thrown if slf4j-log4j and log4j 1.2.17 are on the classpath? In my repo with the reproduction if I edit the gradle file and comment out reload4j and uncomment log4j there's no error thrown so I don't seem to have the behavior described. It's an easy enough fix to just upgrade slf4j but I suppose I'm just wondering why the docs say it has more to do with the jdk version when in my repo there's no error when using log4j.

You are right. Thank you for following up on this.

When I create the classpath manually, the observe behaviors are indeed different when run with log4j.jar and reload4j.jar. Now that I think of it, this actually makes sense, since the type of MDC package private field tlm was changed from Object (in log4j) to ThreadLocal (in reload4j).

This could be just an interaction with the specific versions I'm using, but I just want to make sure we're on the same page

No, mystery solved as explained above. I'll update the docs again (tomorrow).

ceki commented 2 years ago

SLF4J documentation updated to explain the problem.

See https://www.slf4j.org/codes.html#no_tlm

ceki commented 2 years ago

This backward compatibility problem was fixed in commits 0cbed4256a and dc3cc1f24 shipping with reload4j version 1.2.21.

Reload4j version slf4j-reload4j version slf4j-log4j12 version result
1.2.20 17.33 absent OK
1.2.20 absent 1.7.30 exception (see below)
1.2.21 17.33 absent OK
1.2.21 absent 1.7.30 OK 
Exception in thread "main" java.lang.NoSuchFieldError: tlm
        at org.apache.log4j.MDCFriend.fixForJava9(MDCFriend.java:11)
        at org.slf4j.impl.Log4jMDCAdapter.<clinit>(Log4jMDCAdapter.java:38)
        at org.slf4j.impl.StaticMDCBinder.getMDCA(StaticMDCBinder.java:59)
        at org.slf4j.MDC.bwCompatibleGetMDCAdapterFromBinder(MDC.java:99)
        at org.slf4j.MDC.<clinit>(MDC.java:108)
OlivierJaquemet commented 2 years ago

@ceki As you indicated in your last message, this backward compatibility problem was fixed in commits https://github.com/qos-ch/reload4j/commit/0cbed4256a40a9e30e2157997cd137fae50f4c57 and https://github.com/qos-ch/reload4j/commit/dc3cc1f245c0cab397ccdf0bf2a416b53b9cee98 . Version 1.2.21 including those commit has been released.

Therefore, I think this issue can be closed. Correct ?