Prevent thread and object leak

[jdpgrailsdev]

@bluedenim We discovered two more leaks when using the appender with a router:

1. The number of registered shutdown hooks grows unbounded as new appender instances are created
2. The queue of LoggingEventCache instances grows unbounded as new appender instances are created

This PR attempts to fix both by:

• De-registering the shutdown hook thread when the stop() method is called by Log4j
• Removing the cache from the queue when the stop() method on the LoggingEventCache is called as part of the appender stop flow implemented previously.

I do wonder if the shutdown hook is even necessary at this point, even in the standard single appender creation flow. Might be worth doing some testing to see if the stop() implementation makes the shutdown hook unnecessary, though it may still be a good safety net when an application crashes and Log4j cannot complete its shutdown flow.

[bluedenim]

instances is a static final already set, so it wouldn't be null.

It might help to also do this in the dtor (or as close to one Java has):

    @Override
    protected void finalize() {
        try {
            instances.remove(this);
        } catch (IOException ex) {
            VansLogger.logger.error(
                String.format("LoggingEventCache %s cannot remove itself from instances: %s", this, ex)
            );
        }
    }

[jdpgrailsdev]

@bluedenim Forgive the lag in my reply (I was on vacation). I can remove the if check if you think it is worth doing. As for putting the logic in the finalize() method, that is a bit of an anti-pattern in Java. You don't know when finalize() will be called -- it happens when the JVM decides it needs to perform garbage collection, which may not be any time near to when the object is no longer used by the code. This means that in this case, it would not prevent the leak in all scenarios -- it would totally be dependent on the heap size and how much is in use. Hooking into Log4j2's lifecycle is the better approach IMHO as it will get called when Log4j2 stops the appender.

[bluedenim]

Definitely keep the current code. I was proposing to ALSO put it in the finalize(). But it's not that crucial for the reason you mentioned.

The if test is also not a big deal; it's just not necessary.

[jdpgrailsdev]

I think it makes sense to leave it as it is, given that finalize() is deprecated and is to be removed in a future JDK release. Suggested alternatives are to use try-with-resources (we would need to make the cache implement AutoCloseable) or to use the Cleaner API.

[bluedenim]

Sounds good. I will merge and do a release in the next day or 2

[bluedenim]

This is now in release 5.2.2. Thanks for contributing!