Skip to content

Commit

Permalink
Prevent thread and object leak (#149)
Browse files Browse the repository at this point in the history
* Prevent thread and object leak

* Formatting

* Avoid exception on shutdown

* Improve exception handling on shutdown

* Fix compile error
  • Loading branch information
jdpgrailsdev authored Apr 26, 2024
1 parent 8f6d33b commit 4029fed
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,11 @@ public boolean stop() {
cacheMonitor.shutDown();
}

if (null != instances) {
// Remove the stopped cache from the queue to avoid a leak
instances.remove(this);
}

return success;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
public class Log4j2Appender extends AbstractAppender {

private final LoggingEventCache eventCache;
private final Thread shutdownHook;
private boolean verbose = false;

@PluginBuilderFactory
Expand All @@ -38,7 +39,7 @@ public static org.apache.logging.log4j.core.util.Builder<Log4j2Appender> newBuil
super(name, filter, layout, ignoreExceptions);
Objects.requireNonNull(eventCache);
this.eventCache = eventCache;
Runtime.getRuntime().addShutdownHook(new Thread(() -> {
this.shutdownHook = new Thread(() -> {
try {
close();
LoggingEventCache.shutDown();
Expand All @@ -47,7 +48,8 @@ public static org.apache.logging.log4j.core.util.Builder<Log4j2Appender> newBuil
VansLogger.logger.error("InterruptedException during LoggingEventCache.shutDown", e);
}
}
}));
});
Runtime.getRuntime().addShutdownHook(this.shutdownHook);
}

public Log4j2Appender setVerbose(boolean verbose) {
Expand Down Expand Up @@ -76,7 +78,7 @@ public void stop() {
}

Event mapToEvent(LogEvent event) {
String message = null;
String message;
if (null != getLayout()) {
message = getLayout().toSerializable(event).toString();
} else {
Expand All @@ -97,6 +99,19 @@ private void close() {
if (this.verbose) {
VansLogger.logger.info("Publishing staging log on close...");
}

// Deregister shutdown hook to avoid unbounded growth of registered hooks
try {
Runtime.getRuntime().removeShutdownHook(this.shutdownHook);
} catch (IllegalStateException e) {
// Swallow the exception
VansLogger.logger.warn("Already shutting down. Cannot remove shutdown hook.");
} catch (final Exception e) {
// Swallow the exception cause
VansLogger.logger.error("Error while removing shutdown hook", e);
}

// Flush and stop the cache associated with this appender
eventCache.flushAndPublish(true);
eventCache.stop();
}
Expand Down

0 comments on commit 4029fed

Please sign in to comment.