On Jan 19, 2016, at 8:58 AM, Marcus Larsson <marcus.lars...@oracle.com> wrote: > > Hi, > > Please review the following patch to fix an issue in UL causing the VM to > crash during shutdown. > > The problem is that the static LogStdoutOutput is deinitialized before the > last use of it (G1 concurrent thread tries to log very late during VM > shutdown). The solution is to make sure neither LogStdoutOutput nor > LogStderrOutput are deinitialized during the full lifetime of the VM. To > accomplish this I've changed the storage from static objects to static > pointers to heap instances that are allocated & initialized on first use [0]. > These instances are never deleted and can always be used. Also updated > LogConfiguration::finalize to disable all file outputs before deleting & > freeing them. > > Webrev: > http://cr.openjdk.java.net/~mlarsson/8146009/webrev.00/ > > Issue: > https://bugs.openjdk.java.net/browse/JDK-8146009 > > Testing: > - local runs of the reproducer (500+ iterations without crashing) > - JPRT > > Thanks, > Marcus > > [0]: https://isocpp.org/wiki/faq/ctors#static-init-order-on-first-use
------------------------------------------------------------------------------ src/share/vm/logging/logOutput.cpp 32 LogOutput* LogOutput::stdout_output() { 33 static LogOutput* instance = new LogStdoutOutput(); 34 return instance; 35 } 36 37 LogOutput* LogOutput::stderr_output() { 38 static LogOutput* instance = new LogStderrOutput(); 39 return instance; 40 } Function scoped statics may not be thread-safe. They are thread-safe by default for gcc4.x (there's a compiler option to disable that). I'm pretty sure they are not for older versions of Visual Studio, though C++11 requires thread-safe initialization so some relatively recent version of VS probably makes them so. I don't know about Solaris or other platforms. ------------------------------------------------------------------------------ The failure situation this change is attempting to address may still technically invoke undefined behavior with the change. I think that referencing the function scoped static during exit processing may be theoretically problematic because the variable may have been destroyed. It likely works despite that, since the destructor for a pointer-typed value probably trivially does nothing. This suggests that even thread-safe statics may not make the stdxxx_output functions strictly correct. ------------------------------------------------------------------------------ Why do we still have gc (or any other) threads running during a shutdown that does exit processing[1]? That seems like the real bug here. Shouldn't we either have stopped all threads before exit processing, or not be doing exit processing at all and instead be aborting? [1] By exit processing I mean the distinction between exit(3) et al and abort(3). ------------------------------------------------------------------------------