Hi Kim,
On 01/19/2016 08:25 PM, Kim Barrett wrote:
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.
I don't think this will be a problem in this specific case. Both
functions will always be called during early init when there's only a
single thread around (LogConfiguration::initialize and LogTagSet
constructor). I was actually able to limit the usage of these functions
to only these two cases. I'll post a new webrev soon.
------------------------------------------------------------------------------
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.
The functions return copies of the pointers, so no one will really have
a reference to those specific pointers. The memory the pointers point to
is also never freed. With the changes I mentioned above, no one will
call these functions or look at that static data once initialization is
complete. The functions will just be a mean to get predictable static
initialization for stdout and stderr outputs.
------------------------------------------------------------------------------
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).
That's a fair point, and it could indeed be a bug itself. The reason I
chose to fix the logging side of this is that I think it would be good
if the log framework is available at all times, even right before the VM
dies.
Thanks,
Marcus
------------------------------------------------------------------------------
