On Jan 19, 2016, at 8:58 AM, Marcus Larsson <[email protected]> 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).
------------------------------------------------------------------------------
