On 22/02/2019 9:12 pm, Aleksey Shipilev wrote:
On 2/22/19 11:48 AM, David Holmes wrote:
diff -r e94ed0236046 src/hotspot/os/linux/os_linux.cpp
--- a/src/hotspot/os/linux/os_linux.cpp Fri Feb 22 09:23:37 2019 +0100
+++ b/src/hotspot/os/linux/os_linux.cpp Fri Feb 22 10:51:11 2019 +0100
@@ -1357,11 +1357,11 @@
// called from signal handler. Before adding something to os::abort(), make
// sure it is async-safe and can handle partially initialized VM.
void os::abort(bool dump_core, void* siginfo, const void* context) {
os::shutdown();
if (dump_core) {
-#ifndef ZERO
+#ifndef INCLUDE_CDS
if (DumpPrivateMappingsInCore) {
ClassLoader::close_jrt_image();
}
#endif
#ifndef PRODUCT
JDK-8219414 incorrectly removed INCLUDE_CDS guard, while
ClassLoader::close_jrt_image() is still
only defined under INCLUDE_CDS.
Right - though the function seems to be declared unconditionally, the
definition is only under
INCLUDE_CDS. I recall seeing some discussion of this change but am left
wondering how on earth ZERO
came into play! ??
With our current rich feature flags ifdef-ing the JVM variant should be
considered the big red flag.
The code should be guarded with feature flags. And indeed it was, until reviews
got side-tracked:
https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-February/032756.html
Right. Still not sure why ZERO was raised here.
Your fix seems good, but I think there is more work related to
close_jrt_image() needed (by others).
I agree. Not sure if close_jrt_image() should just be NOT_CDS_RETURN, if it is
indeed CDS-specific.
Consider this fix trivial?
:) Sure.
Thanks,
David
-----
-Aleksey
