vy commented on code in PR #3199:
URL: https://github.com/apache/logging-log4j2/pull/3199#discussion_r1843325702
##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java:
##########
@@ -43,7 +41,7 @@
@NullMarked
public class LoggerRegistry<T extends ExtendedLogger> {
- private final Map<String, Map<MessageFactory, WeakReference<T>>>
loggerRefByMessageFactoryByName = new HashMap<>();
+ private final Map<MessageFactory, Map<String, T>>
loggerByNameByMessageFactory = new WeakHashMap<>();
Review Comment:
> Currenlty the `Logger` is GC-ed between the call to `putIfAbsent()` and
the call to `getLogger()`. Unless I am mistaken, `reachabilityFence` will not
help with that.
No, it will. GC kicks in _after_ the `return logger`, i.e., _outside the
scope_ where the reference returned by `WeakReference#get()` is assigned to a
variable. Creating a strong reference to `logger` again within this scope will
solve the problem – I just don't know of a stable way to this without using the
reachability fence. This is in detailed explained
[here](https://shipilev.net/jvm/anatomy-quarks/8-local-var-reachability/). You
can even verify this behavior by creating a `private static final Set<Logger>
LOGGERS` and calling `LOGGERS.add(logger)` just before every `return logger`
statement.
> Therefore the refactoring from `Map<String, Map<MessageFactory,
WeakReference<T>>>` to `Map<String, Map<MessageFactory, T>>` is required.
_Required_? I am not sure. As I stated, I am checking this. Could you give
me a day, please?
> The refactoring from `Map<String, Map<MessageFactory, T>>` to
`Map<MessageFactory, Map<String, T>>` is indeed not needed. Do you object to
this change?
If this is indeed the only way to fix the problem, I don't object it. Please
give me a day.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
