Skip to content

Commit 207a7c3

Browse files
Suvrat1629jhl221123
authored andcommitted
added the comments again
1 parent f53251c commit 207a7c3

File tree

1 file changed

+51
-3
lines changed

1 file changed

+51
-3
lines changed

log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/InternalLoggerRegistry.java

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,12 @@
4141
/**
4242
* A registry of {@link Logger}s namespaced by name and message factory.
4343
* This class is internally used by {@link LoggerContext}.
44-
*
45-
* Handles automatic cleanup of stale logger references to prevent memory leaks.
44+
* <p>
45+
* We don't use {@linkplain org.apache.logging.log4j.spi.LoggerRegistry the
46+
* registry from Log4j API} to keep Log4j Core independent from the version of
47+
* Log4j API at runtime.
48+
* This also allows Log4j Core to evolve independently from Log4j API.
49+
* </p>
4650
*
4751
* @since 2.25.0
4852
*/
@@ -85,6 +89,10 @@ private void removeLogger(Reference<? extends Logger> loggerRef) {
8589

8690
/**
8791
* Returns the logger associated with the given name and message factory.
92+
*
93+
* @param name a logger name
94+
* @param messageFactory a message factory
95+
* @return the logger associated with the given name and message factory
8896
*/
8997
public @Nullable Logger getLogger(final String name, final MessageFactory messageFactory) {
9098
requireNonNull(name, "name");
@@ -112,6 +120,9 @@ public Collection<Logger> getLoggers() {
112120

113121
readLock.lock();
114122
try {
123+
// Return a new collection to allow concurrent iteration over the loggers
124+
//
125+
// https://github.com/apache/logging-log4j2/issues/3234
115126
return loggerRefByNameByMessageFactory.values().stream()
116127
.flatMap(loggerRefByName -> loggerRefByName.values().stream())
117128
.flatMap(loggerRef -> {
@@ -124,12 +135,27 @@ public Collection<Logger> getLoggers() {
124135
}
125136
}
126137

138+
/**
139+
* Checks if a logger associated with the given name and message factory exists.
140+
*
141+
* @param name a logger name
142+
* @param messageFactory a message factory
143+
* @return {@code true}, if the logger exists; {@code false} otherwise.
144+
*/
127145
public boolean hasLogger(final String name, final MessageFactory messageFactory) {
128146
requireNonNull(name, "name");
129147
requireNonNull(messageFactory, "messageFactory");
130148
return getLogger(name, messageFactory) != null;
131149
}
132150

151+
/**
152+
* Checks if a logger associated with the given name and message factory type
153+
* exists.
154+
*
155+
* @param name a logger name
156+
* @param messageFactoryClass a message factory class
157+
* @return {@code true}, if the logger exists; {@code false} otherwise.
158+
*/
133159
public boolean hasLogger(final String name, final Class<? extends MessageFactory> messageFactoryClass) {
134160
requireNonNull(name, "name");
135161
requireNonNull(messageFactoryClass, "messageFactoryClass");
@@ -161,10 +187,30 @@ public Logger computeIfAbsent(
161187
return logger;
162188
}
163189

190+
// Intentionally moving the logger creation outside the write lock, because:
191+
//
192+
// - Logger instantiation is expensive (causes contention on the write-lock)
193+
//
194+
// - User code might have circular code paths, though through different threads.
195+
// Consider `T1[ILR:computeIfAbsent] -> ... -> T1[Logger::new] -> ... ->
196+
// T2[ILR::computeIfAbsent]`.
197+
// Hence, having logger instantiation while holding a write lock might cause
198+
// deadlocks:
199+
// https://github.com/apache/logging-log4j2/issues/3252
200+
// https://github.com/apache/logging-log4j2/issues/3399
201+
//
202+
// - Creating loggers without a lock, allows multiple threads to create loggers
203+
// in parallel, which also improves
204+
// performance.
205+
//
206+
// Since all loggers with the same parameters are equivalent, we can safely
207+
// return the logger from the
208+
// thread that finishes first.
164209
Logger newLogger = loggerSupplier.apply(name, messageFactory);
210+
211+
// Report name and message factory mismatch if there are any
165212
final String loggerName = newLogger.getName();
166213
final MessageFactory loggerMessageFactory = newLogger.getMessageFactory();
167-
168214
if (!loggerName.equals(name) || !loggerMessageFactory.equals(messageFactory)) {
169215
StatusLogger.getLogger()
170216
.error(
@@ -178,9 +224,11 @@ public Logger computeIfAbsent(
178224
messageFactory);
179225
}
180226

227+
// Write lock slow path: Insert the logger
181228
writeLock.lock();
182229
try {
183230
Map<String, WeakReference<Logger>> loggerRefByName = loggerRefByNameByMessageFactory.get(messageFactory);
231+
// noinspection Java8MapApi (avoid the allocation of lambda passed to `Map::computeIfAbsent`)
184232
if (loggerRefByName == null) {
185233
loggerRefByNameByMessageFactory.put(messageFactory, loggerRefByName = new HashMap<>());
186234
}

0 commit comments

Comments
 (0)