Update `newMap.numEntries` after copying the old map in `UnmodifiableArrayBackedMap#copyAndPutAll` #2942

LuciferYang · 2024-09-10T17:49:03Z

This pr fixes a bug in the UnmodifiableArrayBackedMap#copyAndPutAll method by adding an update to newMap's numEntries after copying the contents of the old map.

To reproduce this issue, this pr refactors the test case UnmodifiableArrayBackedMapTest#testCopyAndPut:

run

mvn clean install -pl log4j-api-test -am -DskipTests
mvn test -pl log4j-api-test -Dtest=org.apache.logging.log4j.internal.map.UnmodifiableArrayBackedMapTest

before:

[INFO] Running org.apache.logging.log4j.internal.map.UnmodifiableArrayBackedMapTest
[ERROR] Tests run: 20, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.032 s <<< FAILURE! -- in org.apache.logging.log4j.internal.map.UnmodifiableArrayBackedMapTest
[ERROR] org.apache.logging.log4j.internal.map.UnmodifiableArrayBackedMapTest.testCopyAndPut -- Time elapsed: 0.012 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <null> but was: <another value>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
	at org.apache.logging.log4j.internal.map.UnmodifiableArrayBackedMapTest.testCopyAndPut(UnmodifiableArrayBackedMapTest.java:70)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   UnmodifiableArrayBackedMapTest.testCopyAndPut:70 expected: <null> but was: <another value>
[INFO] 
[ERROR] Tests run: 20, Failures: 1, Errors: 0, Skipped: 0
[INFO]

after:

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.logging.log4j.internal.map.UnmodifiableArrayBackedMapTest
[INFO] Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.028 s -- in org.apache.logging.log4j.internal.map.UnmodifiableArrayBackedMapTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 20, Failures: 0, Errors: 0, Skipped: 0

Checklist

Base your changes on 2.x branch if you are targeting Log4j 2; use main otherwise
./mvnw verify succeeds (if it fails due to code formatting issues reported by Spotless, simply run ./mvnw spotless:apply and retry)
Non-trivial changes contain an entry file in the src/changelog/.2.x.x directory
Tests for the changes are provided
Commits are signed (optional, but highly recommended)

LuciferYang · 2024-09-10T18:04:03Z

locally run ./mvnw verify, all passed

[INFO] Reactor Summary for Apache Log4j BOM 2.25.0-SNAPSHOT:
[INFO] 
[INFO] Apache Log4j BOM ................................... SUCCESS [  7.118 s]
[INFO] Apache Log4j Parent ................................ SUCCESS [  0.158 s]
[INFO] Apache Log4j API Java 9 support .................... SUCCESS [  3.366 s]
[INFO] Apache Log4j API ................................... SUCCESS [  9.394 s]
[INFO] Apache Log4j Implementation Java 9 support ......... SUCCESS [  2.209 s]
[INFO] Apache Log4j Core .................................. SUCCESS [ 21.173 s]
[INFO] Apache Log4j API Tests ............................. SUCCESS [ 13.274 s]
[INFO] Apache Log4j Core Tests ............................ SUCCESS [01:45 min]
[INFO] Apache Log4j 1.x Compatibility API ................. SUCCESS [ 14.776 s]
[INFO] Apache Log4j App Server Support .................... SUCCESS [  2.461 s]
[INFO] Log4j API to SLF4J Adapter ......................... SUCCESS [  4.447 s]
[INFO] SLF4J 1 Binding for Log4j API ...................... SUCCESS [  5.636 s]
[INFO] Apache Log4j Cassandra ............................. SUCCESS [  4.358 s]
[INFO] Apache Log4j Core Integration Tests ................ SUCCESS [  1.267 s]
[INFO] Apache Log4j CouchDB ............................... SUCCESS [  3.229 s]
[INFO] Apache Log4j Docker Library ........................ SUCCESS [  2.801 s]
[INFO] Apache Log4j Streaming Interface ................... SUCCESS [  7.028 s]
[INFO] Apache Log4j Jakarta SMTP .......................... SUCCESS [  4.391 s]
[INFO] Apache Log4j Jakarta Web ........................... SUCCESS [  5.252 s]
[INFO] Apache Log4j Commons Logging Bridge ................ SUCCESS [  3.727 s]
[INFO] Apache Log4j JPA ................................... SUCCESS [  6.073 s]
[INFO] Apache Log4j JDK Platform Logging Adapter .......... SUCCESS [  3.870 s]
[INFO] Apache Log4j JDBC DBCP 2 ........................... SUCCESS [  3.881 s]
[INFO] Apache Log4j JUL Adapter ........................... SUCCESS [  9.119 s]
[INFO] Apache Log4j JSON Template Layout .................. SUCCESS [  5.722 s]
[INFO] Apache Log4j JSON Template Layout tests ............ SUCCESS [ 10.862 s]
[INFO] Apache Log4j MongoDB 4 ............................. SUCCESS [ 22.036 s]
[INFO] Apache Log4j MongoDB Appender ...................... SUCCESS [ 10.958 s]
[INFO] Apache Log4j to JUL Bridge ......................... SUCCESS [  3.821 s]
[INFO] Apache Log4j OSGi tests ............................ SUCCESS [  3.111 s]
[INFO] Apache Log4J Performance Tests ..................... SUCCESS [ 14.975 s]
[INFO] SLF4J 2 Provider for Log4j API ..................... SUCCESS [  6.226 s]
[INFO] Apache Log4j Spring Boot Support ................... SUCCESS [  5.089 s]
[INFO] Apache Log4j Spring Cloud Config Client Support .... SUCCESS [  3.301 s]
[INFO] Apache Log4j Web ................................... SUCCESS [  5.566 s]
[INFO] Apache Log4j Tag Library ........................... SUCCESS [  4.972 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  05:41 min
[INFO] Finished at: 2024-09-11T02:02:06+08:00
[INFO] ------------------------------------------------------------------------

LuciferYang · 2024-09-10T18:04:39Z

cc @jengebr FYI

jengebr · 2024-09-10T18:06:00Z

Thank you! :)

github-actions · 2024-09-10T18:15:40Z

Job	Requested goals	Build Tool Version	Build Outcome
build-macos-latest	clean install	3.9.8	✅
build-ubuntu-latest	clean install	3.9.8	✅
build-windows-latest	clean install	3.9.8	✅

Generated by gradle/develocity-actions

ppkarwasz

Nice catch! 💯

LGTM, I'll schedule it for 2.24.1 in about two weeks.

LuciferYang · 2024-09-13T02:35:45Z

Thanks @ppkarwasz @vy and @jengebr ~

ppkarwasz · 2024-09-13T07:09:25Z

Closes #2946

### What changes were proposed in this pull request? The pr aims to upgrade log4j2 from `2.22.1` to `2.24.1`. ### Why are the changes needed? - The full release notes: https://github.com/apache/logging-log4j2/releases/tag/rel%2F2.24.1 https://github.com/apache/logging-log4j2/releases/tag/rel%2F2.24.0 https://github.com/apache/logging-log4j2/releases/tag/rel%2F2.23.1 https://github.com/apache/logging-log4j2/releases/tag/rel%2F2.23.0 - The new version contains some bug fixes: Fix regression in JdkMapAdapterStringMap performance. (apache/logging-log4j2#2238) Fix NPE in PatternProcessor for a UNIX_MILLIS pattern (apache/logging-log4j2#2346) Fix that parameterized message formatting throws an exception when there are insufficient number of parameters. It previously simply didn't replace the '{}' sequence. The behavior changed in 2.21.0 and should be restored for backward compatibility. (apache/logging-log4j2#2380) Fix putAll() in the default thread context map implementation (apache/logging-log4j2#2942) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #48029 from panbingkun/SPARK-49541. Authored-by: panbingkun <panbingkun@baidu.com> Signed-off-by: yangjie01 <yangjie01@baidu.com>

### What changes were proposed in this pull request? The pr aims to upgrade log4j2 from `2.22.1` to `2.24.1`. ### Why are the changes needed? - The full release notes: https://github.com/apache/logging-log4j2/releases/tag/rel%2F2.24.1 https://github.com/apache/logging-log4j2/releases/tag/rel%2F2.24.0 https://github.com/apache/logging-log4j2/releases/tag/rel%2F2.23.1 https://github.com/apache/logging-log4j2/releases/tag/rel%2F2.23.0 - The new version contains some bug fixes: Fix regression in JdkMapAdapterStringMap performance. (apache/logging-log4j2#2238) Fix NPE in PatternProcessor for a UNIX_MILLIS pattern (apache/logging-log4j2#2346) Fix that parameterized message formatting throws an exception when there are insufficient number of parameters. It previously simply didn't replace the '{}' sequence. The behavior changed in 2.21.0 and should be restored for backward compatibility. (apache/logging-log4j2#2380) Fix putAll() in the default thread context map implementation (apache/logging-log4j2#2942) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#48029 from panbingkun/SPARK-49541. Authored-by: panbingkun <panbingkun@baidu.com> Signed-off-by: yangjie01 <yangjie01@baidu.com>

init

c1a62e9

LuciferYang referenced this pull request Sep 10, 2024

Creating faster ThreadContextMap based on a String[] in the ThreadLocal

b26bb88

LuciferYang mentioned this pull request Sep 10, 2024

[SPARK-49541][BUILD] Upgrade log4j2 to 2.24.1 apache/spark#48029

Closed

ppkarwasz approved these changes Sep 10, 2024

View reviewed changes

ppkarwasz added this to the 2.24.1 milestone Sep 10, 2024

vy added bug Incorrect, unexpected, or unintended behavior of existing code api Affects the public API labels Sep 12, 2024

Add changelog entry

408447b

ppkarwasz mentioned this pull request Sep 12, 2024

(Closeable)ThreadContext putAll removes entire context #2946

Closed

ppkarwasz merged commit 96f5fe7 into apache:2.x Sep 12, 2024
9 checks passed

ppkarwasz removed this from the 2.24.1 milestone Sep 13, 2024

vy mentioned this pull request Jan 31, 2025

Suspect deadlock in InternalLoggerRegistry (intermittent) #3399

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `newMap.numEntries` after copying the old map in `UnmodifiableArrayBackedMap#copyAndPutAll` #2942

Update `newMap.numEntries` after copying the old map in `UnmodifiableArrayBackedMap#copyAndPutAll` #2942

LuciferYang commented Sep 10, 2024 •

edited

Loading

LuciferYang commented Sep 10, 2024

LuciferYang commented Sep 10, 2024

jengebr commented Sep 10, 2024

github-actions bot commented Sep 10, 2024 •

edited

Loading

ppkarwasz left a comment

LuciferYang commented Sep 13, 2024

ppkarwasz commented Sep 13, 2024

Update newMap.numEntries after copying the old map in UnmodifiableArrayBackedMap#copyAndPutAll #2942

Update newMap.numEntries after copying the old map in UnmodifiableArrayBackedMap#copyAndPutAll #2942

Conversation

LuciferYang commented Sep 10, 2024 • edited Loading

Checklist

LuciferYang commented Sep 10, 2024

LuciferYang commented Sep 10, 2024

jengebr commented Sep 10, 2024

github-actions bot commented Sep 10, 2024 • edited Loading

Generated by gradle/develocity-actions

ppkarwasz left a comment

Choose a reason for hiding this comment

LuciferYang commented Sep 13, 2024

ppkarwasz commented Sep 13, 2024

Update `newMap.numEntries` after copying the old map in `UnmodifiableArrayBackedMap#copyAndPutAll` #2942

Update `newMap.numEntries` after copying the old map in `UnmodifiableArrayBackedMap#copyAndPutAll` #2942

LuciferYang commented Sep 10, 2024 •

edited

Loading

github-actions bot commented Sep 10, 2024 •

edited

Loading