Looks good to me. Thanks for taking care of this, Aleksey!
- Jini.
On 2/22/2019 5:56 PM, Aleksey Shipilev wrote:
On 2/22/19 12:19 PM, Jini George wrote:
JDK-8219414 decoupled CDS and DumpPrivateMappingsInCore. Removing the
#INCLUDE_CDS guard from the
definition of ClassLoader::close_jrt_image() got missed out (My bad! Missed
that during the review).
So, since ClassLoader::close_jrt_image() is decoupled from CDS, it might be
better to move the
definition of ClassLoader::close_jrt_image() out of the #INCLUDE_CDS guard and
remove the then
un-needed #ifndef ZERO guard from the call to ClassLoader::close_jrt_image(). I
think it would also
mean that the assert ClassLoader::has_jrt_entry() in the definition of
ClassLoader::close_jrt_image() would need to be changed to a return if
(!ClassLoader::has_jrt_entry()) -- else the assert would fail for exploded
builds.
All right, let's do that:
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 13:25:27 2019 +0100
@@ -1357,15 +1357,13 @@
// 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
if (DumpPrivateMappingsInCore) {
ClassLoader::close_jrt_image();
}
-#endif
#ifndef PRODUCT
fdStream out(defaultStream::output_fd());
out.print_raw("Current thread is ");
char buf[16];
jio_snprintf(buf, sizeof(buf), UINTX_FORMAT, os::current_thread_id());
diff -r e94ed0236046 src/hotspot/share/classfile/classLoader.cpp
--- a/src/hotspot/share/classfile/classLoader.cpp Fri Feb 22 09:23:37
2019 +0100
+++ b/src/hotspot/share/classfile/classLoader.cpp Fri Feb 22 13:25:27
2019 +0100
@@ -618,17 +618,18 @@
void ClassLoader::setup_module_search_path(const char* path, TRAPS) {
update_module_path_entry_list(path, THREAD);
}
+#endif // INCLUDE_CDS
+
void ClassLoader::close_jrt_image() {
- assert(ClassLoader::has_jrt_entry(), "Not applicable for exploded builds");
+ // Not applicable for exploded builds
+ if (!ClassLoader::has_jrt_entry()) return;
_jrt_entry->close_jimage();
}
-#endif // INCLUDE_CDS
-
// Construct the array of module/path pairs as specified to --patch-module
// for the boot loader to search ahead of the jimage, if the class being
// loaded is defined to a module that has been specified to --patch-module.
void ClassLoader::setup_patch_mod_entries() {
Thread* THREAD = Thread::current();
Testing: Linux x86_64 {server, zero}, Linux x86 {minimal} builds
-Aleksey
