Potential loss of data on JVM termination

[codefromthecrypt]

From @wjase on May 2, 2018 1:2

Saw a warning from one of our Spring Artefacts on shutdown:

04 Apr 2018 12:36:34,299] WARN  [requestId=] [loader.WebappClassLoaderBase]: The web 
application [XXX] appears to have started a thread named [AsyncReporter(XXX)] but has failed to 
stop it. This is very likely to create a memory leak. Stack trace of thread:\n 
sun.misc.Unsafe.park(Native Method) 
java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:215) java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.awaitNanos(AbstractQueuedSynchronizer.java:2078)
zipkin.reporter.ByteBoundedQueue.drainTo(ByteBoundedQueue.java:83)
zipkin.reporter.AsyncReporter$BoundedAsyncReporter.flush(AsyncReporter.java:227)
zipkin.reporter.AsyncReporter$Builder.lambda$build$0(AsyncReporter.java:166)
zipkin.reporter.AsyncReporter$Builder$$Lambda$1.run(Unknown Source) 
java.lang.Thread.run(Thread.java:745)

I think this is the Daemon thread created in zipkin.reporter.AsyncReporter.build(Encoder<S> encoder)

Normal thread and daemon threads differ in what happens when they exit. When the JVM halts any remaining daemon threads are abandoned: finally blocks are not executed, stacks are not unwound - the JVM just exits. Due to this reason daemon threads should be used sparingly and it is dangerous to use them for tasks that might perform any sort of I/O

The daemon flushing thread could potentially just exit without completion (or executing the finally block), losing trace data. Perhaps the thread should have a shutdown hook and not be set as a daemon thread?

Copied from original issue: openzipkin/zipkin#2042

[codefromthecrypt]

@wjase is your reporter a bean? If so, it should have closed itself in shutdown obviating this message. I think it is important to close the reporter and not rely on shutdown hooks especially in Spring. Please elaborate if there's some reason why your spring config cannot solve this.

[codefromthecrypt]

PS on shutdown hook, there's an interesting conversation stemming from this.. they can get complicated. Not saying we don't do it, but it is interesting https://github.com/census-instrumentation/opencensus-java/pull/1050

Note also that the reporter thread is instance scoped (not necessarily a classloader singleton), so yeah this also affects the choice of doing a shutdown hook approach or rather letting folks do one on their own.

[Logic-32]

My two cents on shutdown hooks: they don't work very well in web applications. For example, running in any servlet container (e.g. Tomcat), if you undeploy a context it doesn't kill the JVM so the shutdown hook doesn't execute. Meaning, any resources relying on it are left dangling and another instance of your app can be deployed, consuming more of the same resources.

It is of course important to note that we don't use Spring for our web apps so any fancy handling it may have does not apply to us.

ServletContextListener.contextDestroyed() is the only reliable way for servlets to clean up resources properly that I'm aware of.

Sorry if this is a redundant comment from the opencensus thread. Don't quite have the time to read through that whole thing at the moment ;)

[codefromthecrypt]

closing this per feedback