Expunge stale loggers in InternalLoggerRegistry on method invocation

[Suvrat1629]

This PR updates InternalLoggerRegistry to automatically remove stale loggers when its methods are invoked.
Changes Introduced:
Introduced a ReferenceQueue to track loggers that have been reclaimed by the garbage collector.
Implemented expungeStaleEntries(), which removes stale loggers before executing registry operations.
Updated methods like getLogger() and computeIfAbsent() to invoke expungeStaleEntries() before proceeding.
Added tests to verify stale loggers are expunged without relying on private methods.

Fixes #3430

[Suvrat1629]

@vy I have always had a hard time with test cases, I have opened a pr regardless please take look and provide further insights on how i can improve it.

[vy]

Please put back all the removed comment blocks.

[Suvrat1629]

Ok sorry about that, I have put back all the comment blocks as they were.

[vy]

Assume I move this logic (and only this!) to a method called unsafeRemoveLogger(Logger). Note that this method receives a Logger, not a Reference.

[vy]

Would you mind changing this loop as follows, please?

boolean locked = false;
Reference<? extends Logger> loggerRef;
while ((loggerRef = staleLoggerRefs.poll()) != null) {
    Logger logger = loggerRef.get();
    if (logger != null) {
        if (!locked) {
            writeLock.lock();
        }
        unsafeRemoveLogger(logger);
    }
}
if (locked) {
    writeLock.unlock();
}

See my other comment regarding the unsafeRemoveLogger(Logger) method.

[ppkarwasz]

Note that if loggerRef.get() returns null, we still need to remove the entry that holds the WeakReference.

What do you think about such an approach:

1. We poll() the queue until its empty.
2. If there was any reference in the queue we iterate once over all the entries of the map and we remove all the invalidated WeakReferences, not just those that were in the queue.

As of my previous comment, this seems a little bit inefficient if loggerRef.get() is not null, but is more efficient than iterating over all entries once-per-reference.

[vy]

@ppkarwasz, you're right. @Suvrat1629, please proceed with the suggestion of @ppkarwasz. Though I suggest having double-checking:

if (!refQueue.isEmpty) {
    lock();
    try {
        if (!refQueue.isEmpty) {
            // Clear `refQueue`
            // Clean up `loggerRefByNameByMessageFactory`
        }
    } finally {
       unlock();
    }
}

[vy]

Can you replace LoggerContext.getContext() calls as follows, please?

@Test
void someTest(TestInfo testInfo) {
    try (final LoggerContext loggerContext = new LoggerContext(testInfo.getDisplayName)) {
        // Use `loggerContext`
    }
}

That is, don't disrupt the global one, use ephemeral LCs instead.

[vy]

Can we instead replace this as follows, please?

1. Use an ephemeral LoggerContext (as I described in another comment)
2. Create 1000 Loggers (should be many enough to create GC pressure)
3. Log using all Loggers (so we imitate a real-world setup where Loggers are used at some point)
4. Use Awaitility.waitAtMost(...).until(...) to verify that after a System.gc(), InternalLoggerRegistry::loggerRefByNameByMessageFactory is emptied

You can access to InternalLoggerRegistry::loggerRefByNameByMessageFactory using reflection. That is, first extract the LoggerContext::loggerRegistry field, and then the InternalLoggerRegistry::loggerRefByNameByMessageFactory field.