Skip to content

Commit 4dee591

Browse files
vpelikhvpelikh
authored
[main] Port InternalLoggerRegistry from 2.x (#3418 and #3681 ports) (#4157)
* Port `InternalLoggerRegistry` from `2.x` Minimizes lock usage by moving logger instantiation outside the write lock (PR #3418). Adds stale entry detection via ReferenceQueue (PR #3681). Signed-off-by: Vasily Pelikh <2010720+vpelikh@users.noreply.github.com> * Apply spotless formatting to InternalLoggerRegistryTest * Fix InternalLoggerRegistry null-MessageFactory lookup mismatch in Log4j 3 InternalLoggerRegistry.getLogger(name, null) normalized null to a hardcoded ParameterizedMessageFactory.INSTANCE, but LoggerContext stored loggers under its DI-injected defaultMessageFactory (ReusableMessageFactory in Log4j 3). This store-key/get-key mismatch caused testGetLoggerRetrievesExistingLogger, testHasLoggerReturnsCorrectStatus, and testExpungeStaleWeakReferenceEntries to all fail. - InternalLoggerRegistry: constructor-inject defaultMessageFactory (final field), resolve null→defaultMessageFactory instead of hardcoded INSTANCE. - LoggerContext: construct InternalLoggerRegistry with its own defaultMessageFactory after DI resolution. - Test: iterate all message-factory buckets (.values()) instead of assuming ParameterizedMessageFactory.INSTANCE. * InternalLoggerRegistry: add Javadoc, align with 2.x naming, fix return-in-finally, add expungeStaleEntries --------- Signed-off-by: Vasily Pelikh <2010720+vpelikh@users.noreply.github.com>
1 parent 554d8f9 commit 4dee591

3 files changed

Lines changed: 504 additions & 10 deletions

File tree

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/InternalLoggerRegistryTest.java‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core.util.internal;
18+
19+
import static java.util.concurrent.TimeUnit.MILLISECONDS;
20+
import static java.util.concurrent.TimeUnit.SECONDS;
21+
import static org.awaitility.Awaitility.await;
22+
import static org.junit.jupiter.api.Assertions.assertEquals;
23+
import static org.junit.jupiter.api.Assertions.assertFalse;
24+
import static org.junit.jupiter.api.Assertions.assertNotNull;
25+
import static org.junit.jupiter.api.Assertions.assertNull;
26+
import static org.junit.jupiter.api.Assertions.assertSame;
27+
import static org.junit.jupiter.api.Assertions.assertTrue;
28+
29+
import java.lang.ref.WeakReference;
30+
import java.lang.reflect.Field;
31+
import java.net.URI;
32+
import java.util.Map;
33+
import org.apache.logging.log4j.core.Logger;
34+
import org.apache.logging.log4j.core.LoggerContext;
35+
import org.apache.logging.log4j.message.MessageFactory;
36+
import org.apache.logging.log4j.message.SimpleMessageFactory;
37+
import org.apache.logging.log4j.plugins.di.DI;
38+
import org.junit.jupiter.api.AfterEach;
39+
import org.junit.jupiter.api.BeforeEach;
40+
import org.junit.jupiter.api.Test;
41+
import org.junit.jupiter.api.TestInfo;
42+
43+
class InternalLoggerRegistryTest {
44+
private LoggerContext loggerContext;
45+
private InternalLoggerRegistry registry;
46+
47+
@BeforeEach
48+
void setUp(final TestInfo testInfo) throws Exception {
49+
loggerContext = new LoggerContext(testInfo.getDisplayName(), null, (URI) null, DI.createInitializedFactory());
50+
final Field registryField = LoggerContext.class.getDeclaredField("loggerRegistry");
51+
registryField.setAccessible(true);
52+
registry = (InternalLoggerRegistry) registryField.get(loggerContext);
53+
}
54+
55+
@AfterEach
56+
void tearDown() {
57+
if (loggerContext != null) {
58+
loggerContext.stop();
59+
}
60+
}
61+
62+
@Test
63+
void testGetLoggerReturnsNullForNonExistentLogger() {
64+
assertNull(registry.getLogger("nonExistent", null));
65+
}
66+
67+
@Test
68+
void testComputeIfAbsentCreatesLogger() {
69+
final Logger logger = loggerContext.getLogger("testLogger", null);
70+
assertNotNull(logger);
71+
assertEquals("testLogger", logger.getName());
72+
}
73+
74+
@Test
75+
void testGetLoggerRetrievesExistingLogger() {
76+
final Logger logger = loggerContext.getLogger("testLogger", null);
77+
assertSame(logger, registry.getLogger("testLogger", null));
78+
}
79+
80+
@Test
81+
void testHasLoggerReturnsCorrectStatus() {
82+
assertFalse(registry.hasLogger("testLogger", (MessageFactory) null));
83+
loggerContext.getLogger("testLogger", null);
84+
assertTrue(registry.hasLogger("testLogger", (MessageFactory) null));
85+
}
86+
87+
@Test
88+
void testExpungeStaleWeakReferenceEntries() throws Exception {
89+
final String loggerNamePrefix = "testLogger_";
90+
final int numberOfLoggers = 1000;
91+
92+
for (int i = 0; i < numberOfLoggers; i++) {
93+
final Logger logger = loggerContext.getLogger(loggerNamePrefix + i, null);
94+
logger.info("Using logger {}", logger.getName());
95+
}
96+
97+
await().atMost(10, SECONDS).pollInterval(100, MILLISECONDS).untilAsserted(() -> {
98+
System.gc();
99+
registry.getLogger("triggerExpunge", null);
100+
101+
final Map<MessageFactory, Map<String, WeakReference<Logger>>> loggerRefByNameByMessageFactory =
102+
reflectAndGetLoggerMapFromRegistry();
103+
104+
int unexpectedCount = 0;
105+
for (final Map<String, WeakReference<Logger>> loggerRefByName : loggerRefByNameByMessageFactory.values()) {
106+
for (int i = 0; i < numberOfLoggers; i++) {
107+
if (loggerRefByName.containsKey(loggerNamePrefix + i)) {
108+
unexpectedCount++;
109+
}
110+
}
111+
}
112+
assertEquals(
113+
0, unexpectedCount, "Found " + unexpectedCount + " unexpected stale entries for MessageFactory");
114+
});
115+
}
116+
117+
@Test
118+
void testExpungeStaleMessageFactoryEntry() throws Exception {
119+
final SimpleMessageFactory mockMessageFactory = new SimpleMessageFactory();
120+
Logger logger = loggerContext.getLogger("testLogger", mockMessageFactory);
121+
logger.info("Using logger {}", logger.getName());
122+
logger = null;
123+
124+
await().atMost(10, SECONDS).pollInterval(100, MILLISECONDS).untilAsserted(() -> {
125+
System.gc();
126+
registry.getLogger("triggerExpunge", null);
127+
128+
final Map<MessageFactory, Map<String, WeakReference<Logger>>> loggerRefByNameByMessageFactory =
129+
reflectAndGetLoggerMapFromRegistry();
130+
assertNull(
131+
loggerRefByNameByMessageFactory.get(mockMessageFactory),
132+
"Stale MessageFactory entry was not removed from the outer map");
133+
});
134+
}
135+
136+
private Map<MessageFactory, Map<String, WeakReference<Logger>>> reflectAndGetLoggerMapFromRegistry()
137+
throws NoSuchFieldException, IllegalAccessException {
138+
final Field loggerMapField = InternalLoggerRegistry.class.getDeclaredField("loggerRefByNameByMessageFactory");
139+
loggerMapField.setAccessible(true);
140+
@SuppressWarnings("unchecked")
141+
final Map<MessageFactory, Map<String, WeakReference<Logger>>> loggerMap =
142+
(Map<MessageFactory, Map<String, WeakReference<Logger>>>) loggerMapField.get(registry);
143+
return loggerMap;
144+
}
145+
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.apache.logging.log4j.core.util.ExecutorServices;
5252
import org.apache.logging.log4j.core.util.NetUtils;
5353
import org.apache.logging.log4j.core.util.ShutdownCallbackRegistry;
54+
import org.apache.logging.log4j.core.util.internal.InternalLoggerRegistry;
5455
import org.apache.logging.log4j.kit.env.PropertyEnvironment;
5556
import org.apache.logging.log4j.kit.env.internal.ContextualEnvironmentPropertySource;
5657
import org.apache.logging.log4j.kit.env.internal.ContextualJavaPropsPropertySource;
@@ -88,8 +89,8 @@ public class LoggerContext extends AbstractLifeCycle
8889

8990
public static final Key<LoggerContext> KEY = Key.forClass(LoggerContext.class);
9091

91-
private final LoggerRegistry<Logger> loggerRegistry = new LoggerRegistry<>();
9292
private final MessageFactory defaultMessageFactory;
93+
private final InternalLoggerRegistry loggerRegistry;
9394

9495
private final Collection<Consumer<Configuration>> configurationStartedListeners = new ArrayList<>();
9596
private final Collection<Consumer<Configuration>> configurationStoppedListeners = new ArrayList<>();
@@ -147,6 +148,7 @@ public LoggerContext(
147148
this.environment = instanceFactory.getInstance(PropertyEnvironment.class);
148149
this.configurationScheduler = instanceFactory.getInstance(ConfigurationScheduler.class);
149150
this.defaultMessageFactory = instanceFactory.getInstance(MessageFactory.class);
151+
this.loggerRegistry = new InternalLoggerRegistry(this.defaultMessageFactory);
150152

151153
this.configuration = new DefaultConfiguration(this);
152154
this.nullConfiguration = new NullConfiguration(this);
@@ -571,15 +573,9 @@ public Collection<Logger> getLoggers() {
571573
@Override
572574
public Logger getLogger(final String name, final MessageFactory messageFactory) {
573575
final MessageFactory actualMessageFactory = messageFactory != null ? messageFactory : defaultMessageFactory;
574-
// Note: This is the only method where we add entries to the 'loggerRegistry' ivar.
575-
Logger logger = loggerRegistry.getLogger(name, actualMessageFactory);
576-
if (logger != null) {
577-
checkMessageFactory(logger, actualMessageFactory);
578-
return logger;
579-
}
580-
logger = newLogger(name, actualMessageFactory);
581-
loggerRegistry.putIfAbsent(name, logger.getMessageFactory(), logger);
582-
return loggerRegistry.getLogger(name, logger.getMessageFactory());
576+
final Logger logger = loggerRegistry.computeIfAbsent(name, actualMessageFactory, this::newLogger);
577+
checkMessageFactory(logger, actualMessageFactory);
578+
return logger;
583579
}
584580

585581
/**

0 commit comments

Comments
 (0)