kamon-io/kanela

Memory leak in tinylog

Closed this issue ยท 9 comments

When instrumentation is triggering errors like the below:

An error occurred while trying to apply an advisor: java.lang.NoClassDefFoundError: redis/clients/jedis/commands/ProtocolCommand

These errors are not printed to the console, and instead are causing an memory leak.

I found this via an heapdump:
image

I think the problem is that the writing thread of tinylog is no longer running, becase it is configured to track the "main" thread and stops as soon the "main" thread dies...

val configurator = Configurator
.fromResource("kanela-log.properties")
.writingThread("main")
.maxStackTraceElements(400) // stack traces
.level(config.getLogLevel());

I guess the solution is to manually control the tinylog writing thread as documented here:

https://tinylog.org/v1/configuration#writingthread

This leaves us with a non-daemon thread that prevents graceful JVM exit. How do we retain the previous behavior?

This leaves us with a non-daemon thread that prevents graceful JVM exit. How do we retain the previous behavior?

Did you try this from https://tinylog.org/v1/configuration#writingthread:

For this purpose, the static method Configurator.shutdownWritingThread() can be called

@nadavwr @jtjeferreira I've opened a new issue to correct this behavior and avoid the leak: #138

@dpsoft why don't we completely remove asynchronous logging from Kanela? There really isn't a log being logged in normal operations. In fact, in most applications the only thing we will ever log is the Kanela banner ๐Ÿ˜„

I wouldn't be concerned at all if all the logging coming out of Kanela was synchronous and we forget about these issues.

indeed, in normal operation the log level is warning and we shouldn't log anything, only in case of an unexpected error. +1 to make sync the logger. @jtjeferreira @nadavwr thoughts?

indeed, in normal operation the log level is warning and we shouldn't log anything, only in case of and unexpected error. +1 to make sync the logger. @jtjeferreira @nadavwr thoughts?

๐Ÿ‘

This sounds like the right call -- IMO reliability should override performance considerations for this logger.

This leaves us with a non-daemon thread that prevents graceful JVM exit. How do we retain the previous behavior?

Did you try this from https://tinylog.org/v1/configuration#writingthread:

For this purpose, the static method Configurator.shutdownWritingThread() can be called

We'll stay at Kanela 1.0.13 for now in anticipation of sync logging, but this sounds like a good workaround for whomever comes across the issue. Thanks!

FWIW the dangling non-daemon thread preventing JVM shutdown hit us, and I've spent an hour tracking this down. Look forward to seeing a resolution. The workaround of calling Configurator.shutdownWritingThread() from our application didn't seem to have the desired effect either, not sure why. Reverting to 1.0.13 did fix it though.