From 4b153e5e051c01ad8d0c3ff335352918c2970fe6 Mon Sep 17 00:00:00 2001 From: Matias Saavedra Silva Date: Mon, 24 Jun 2024 18:19:03 +0000 Subject: [PATCH] 8306580: Propagate CDS dumping errors instead of directly exiting the VM Reviewed-by: iklam, ccheung --- src/hotspot/share/cds/archiveBuilder.cpp | 7 -- src/hotspot/share/cds/archiveBuilder.hpp | 3 - src/hotspot/share/cds/filemap.cpp | 5 +- src/hotspot/share/cds/metaspaceShared.cpp | 97 +++++++++++-------- src/hotspot/share/cds/metaspaceShared.hpp | 9 +- src/hotspot/share/runtime/threads.cpp | 2 +- .../jtreg/runtime/cds/StaticWritingError.java | 52 ++++++++++ 7 files changed, 119 insertions(+), 56 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/cds/StaticWritingError.java diff --git a/src/hotspot/share/cds/archiveBuilder.cpp b/src/hotspot/share/cds/archiveBuilder.cpp index 72e45a7998c..a87a3ff042d 100644 --- a/src/hotspot/share/cds/archiveBuilder.cpp +++ b/src/hotspot/share/cds/archiveBuilder.cpp @@ -1412,10 +1412,3 @@ void ArchiveBuilder::report_out_of_space(const char* name, size_t needed_bytes) log_error(cds)("Unable to allocate from '%s' region: Please reduce the number of shared classes.", name); MetaspaceShared::unrecoverable_writing_error(); } - - -#ifndef PRODUCT -void ArchiveBuilder::assert_is_vm_thread() { - assert(Thread::current()->is_VM_thread(), "ArchiveBuilder should be used only inside the VMThread"); -} -#endif diff --git a/src/hotspot/share/cds/archiveBuilder.hpp b/src/hotspot/share/cds/archiveBuilder.hpp index ad0302137f5..c17090ee53d 100644 --- a/src/hotspot/share/cds/archiveBuilder.hpp +++ b/src/hotspot/share/cds/archiveBuilder.hpp @@ -343,8 +343,6 @@ public: return to_offset_u4(offset); } - static void assert_is_vm_thread() PRODUCT_RETURN; - public: ArchiveBuilder(); ~ArchiveBuilder(); @@ -432,7 +430,6 @@ public: } static ArchiveBuilder* current() { - assert_is_vm_thread(); assert(_current != nullptr, "ArchiveBuilder must be active"); return _current; } diff --git a/src/hotspot/share/cds/filemap.cpp b/src/hotspot/share/cds/filemap.cpp index 5b352b54e4b..96c826fb67e 100644 --- a/src/hotspot/share/cds/filemap.cpp +++ b/src/hotspot/share/cds/filemap.cpp @@ -1406,7 +1406,8 @@ void FileMapInfo::open_for_write() { if (fd < 0) { log_error(cds)("Unable to create shared archive file %s: (%s).", _full_path, os::strerror(errno)); - MetaspaceShared::unrecoverable_writing_error(); + MetaspaceShared::writing_error(); + return; } _fd = fd; _file_open = true; @@ -1659,7 +1660,7 @@ void FileMapInfo::write_bytes(const void* buffer, size_t nbytes) { // If the shared archive is corrupted, close it and remove it. close(); remove(_full_path); - MetaspaceShared::unrecoverable_writing_error("Unable to write to shared archive file."); + MetaspaceShared::writing_error("Unable to write to shared archive file."); } _file_offset += nbytes; } diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index 30b240b27ca..a5061fef567 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -444,6 +444,8 @@ void MetaspaceShared::rewrite_nofast_bytecodes_and_calculate_fingerprints(Thread class VM_PopulateDumpSharedSpace : public VM_Operation { private: ArchiveHeapInfo _heap_info; + FileMapInfo* _map_info; + StaticArchiveBuilder& _builder; void dump_java_heap_objects(GrowableArray* klasses) NOT_CDS_JAVA_HEAP_RETURN; void dump_shared_symbol_table(GrowableArray* symbols) { @@ -454,11 +456,14 @@ private: public: - VM_PopulateDumpSharedSpace() : VM_Operation(), _heap_info() {} + VM_PopulateDumpSharedSpace(StaticArchiveBuilder& b) : + VM_Operation(), _heap_info(), _map_info(nullptr), _builder(b) {} bool skip_operation() const { return false; } VMOp_Type type() const { return VMOp_PopulateDumpSharedSpace; } + ArchiveHeapInfo* heap_info() { return &_heap_info; } + FileMapInfo* map_info() const { return _map_info; } void doit(); // outline because gdb sucks bool allow_nested_vm_operations() const { return true; } }; // class VM_PopulateDumpSharedSpace @@ -515,22 +520,21 @@ void VM_PopulateDumpSharedSpace::doit() { MutexLocker ml(DumpTimeTable_lock, Mutex::_no_safepoint_check_flag); SystemDictionaryShared::check_excluded_classes(); - StaticArchiveBuilder builder; - builder.gather_source_objs(); - builder.reserve_buffer(); + _builder.gather_source_objs(); + _builder.reserve_buffer(); - CppVtables::dumptime_init(&builder); + CppVtables::dumptime_init(&_builder); - builder.sort_metadata_objs(); - builder.dump_rw_metadata(); - builder.dump_ro_metadata(); - builder.relocate_metaspaceobj_embedded_pointers(); + _builder.sort_metadata_objs(); + _builder.dump_rw_metadata(); + _builder.dump_ro_metadata(); + _builder.relocate_metaspaceobj_embedded_pointers(); - dump_java_heap_objects(builder.klasses()); - dump_shared_symbol_table(builder.symbols()); + dump_java_heap_objects(_builder.klasses()); + dump_shared_symbol_table(_builder.symbols()); log_info(cds)("Make classes shareable"); - builder.make_klasses_shareable(); + _builder.make_klasses_shareable(); char* serialized_data = dump_read_only_tables(); @@ -540,28 +544,13 @@ void VM_PopulateDumpSharedSpace::doit() { // We don't want to write these addresses into the archive. CppVtables::zero_archived_vtables(); - // relocate the data so that it can be mapped to MetaspaceShared::requested_base_address() - // without runtime relocation. - builder.relocate_to_requested(); - // Write the archive file const char* static_archive = CDSConfig::static_archive_path(); assert(static_archive != nullptr, "SharedArchiveFile not set?"); - FileMapInfo* mapinfo = new FileMapInfo(static_archive, true); - mapinfo->populate_header(MetaspaceShared::core_region_alignment()); - mapinfo->set_serialized_data(serialized_data); - mapinfo->set_cloned_vtables(CppVtables::vtables_serialized_base()); - mapinfo->open_for_write(); - builder.write_archive(mapinfo, &_heap_info); - - if (PrintSystemDictionaryAtExit) { - SystemDictionary::print(); - } - - if (AllowArchivingWithJavaAgent) { - log_warning(cds)("This archive was created with AllowArchivingWithJavaAgent. It should be used " - "for testing purposes only and should not be used in a production environment"); - } + _map_info = new FileMapInfo(static_archive, true); + _map_info->populate_header(MetaspaceShared::core_region_alignment()); + _map_info->set_serialized_data(serialized_data); + _map_info->set_cloned_vtables(CppVtables::vtables_serialized_base()); } class CollectCLDClosure : public CLDClosure { @@ -663,21 +652,19 @@ void MetaspaceShared::prepare_for_dumping() { // Preload classes from a list, populate the shared spaces and dump to a // file. -void MetaspaceShared::preload_and_dump() { - EXCEPTION_MARK; +void MetaspaceShared::preload_and_dump(TRAPS) { ResourceMark rm(THREAD); - preload_and_dump_impl(THREAD); + StaticArchiveBuilder builder; + preload_and_dump_impl(builder, THREAD); if (HAS_PENDING_EXCEPTION) { if (PENDING_EXCEPTION->is_a(vmClasses::OutOfMemoryError_klass())) { log_error(cds)("Out of memory. Please run with a larger Java heap, current MaxHeapSize = " SIZE_FORMAT "M", MaxHeapSize/M); - CLEAR_PENDING_EXCEPTION; - MetaspaceShared::unrecoverable_writing_error(); + MetaspaceShared::writing_error(); } else { log_error(cds)("%s: %s", PENDING_EXCEPTION->klass()->external_name(), java_lang_String::as_utf8_string(java_lang_Throwable::message(PENDING_EXCEPTION))); - CLEAR_PENDING_EXCEPTION; - MetaspaceShared::unrecoverable_writing_error("VM exits due to exception, use -Xlog:cds,exceptions=trace for detail"); + MetaspaceShared::writing_error("Unexpected exception, use -Xlog:cds,exceptions=trace for detail"); } } } @@ -768,7 +755,7 @@ void MetaspaceShared::preload_classes(TRAPS) { log_info(cds)("Loading classes to share: done."); } -void MetaspaceShared::preload_and_dump_impl(TRAPS) { +void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS) { preload_classes(CHECK); if (SharedArchiveConfigFile) { @@ -805,8 +792,30 @@ void MetaspaceShared::preload_and_dump_impl(TRAPS) { } #endif - VM_PopulateDumpSharedSpace op; + VM_PopulateDumpSharedSpace op(builder); VMThread::execute(&op); + + if (!write_static_archive(&builder, op.map_info(), op.heap_info())) { + THROW_MSG(vmSymbols::java_io_IOException(), "Encountered error while dumping"); + } +} + +bool MetaspaceShared::write_static_archive(ArchiveBuilder* builder, FileMapInfo* map_info, ArchiveHeapInfo* heap_info) { + // relocate the data so that it can be mapped to MetaspaceShared::requested_base_address() + // without runtime relocation. + builder->relocate_to_requested(); + + map_info->open_for_write(); + if (!map_info->is_open()) { + return false; + } + builder->write_archive(map_info, heap_info); + + if (AllowArchivingWithJavaAgent) { + log_warning(cds)("This archive was created with AllowArchivingWithJavaAgent. It should be used " + "for testing purposes only and should not be used in a production environment"); + } + return true; } // Returns true if the class's status has changed. @@ -916,11 +925,17 @@ void MetaspaceShared::unrecoverable_loading_error(const char* message) { // This function is called when the JVM is unable to write the specified CDS archive due to an // unrecoverable error. void MetaspaceShared::unrecoverable_writing_error(const char* message) { + writing_error(message); + vm_direct_exit(1); +} + +// This function is called when the JVM is unable to write the specified CDS archive due to a +// an error. The error will be propagated +void MetaspaceShared::writing_error(const char* message) { log_error(cds)("An error has occurred while writing the shared archive file."); if (message != nullptr) { log_error(cds)("%s", message); } - vm_direct_exit(1); } void MetaspaceShared::initialize_runtime_shared_and_meta_spaces() { diff --git a/src/hotspot/share/cds/metaspaceShared.hpp b/src/hotspot/share/cds/metaspaceShared.hpp index 1fb6ae88142..f26af21676a 100644 --- a/src/hotspot/share/cds/metaspaceShared.hpp +++ b/src/hotspot/share/cds/metaspaceShared.hpp @@ -31,9 +31,12 @@ #include "oops/oop.hpp" #include "utilities/macros.hpp" +class ArchiveBuilder; +class ArchiveHeapInfo; class FileMapInfo; class outputStream; class SerializeClosure; +class StaticArchiveBuilder; template class GrowableArray; @@ -66,13 +69,13 @@ class MetaspaceShared : AllStatic { }; static void prepare_for_dumping() NOT_CDS_RETURN; - static void preload_and_dump() NOT_CDS_RETURN; + static void preload_and_dump(TRAPS) NOT_CDS_RETURN; #ifdef _LP64 static void adjust_heap_sizes_for_dumping() NOT_CDS_JAVA_HEAP_RETURN; #endif private: - static void preload_and_dump_impl(TRAPS) NOT_CDS_RETURN; + static void preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS) NOT_CDS_RETURN; static void preload_classes(TRAPS) NOT_CDS_RETURN; public: @@ -105,6 +108,7 @@ public: static void unrecoverable_loading_error(const char* message = nullptr); static void unrecoverable_writing_error(const char* message = nullptr); + static void writing_error(const char* message = nullptr); static void serialize(SerializeClosure* sc) NOT_CDS_RETURN; @@ -166,6 +170,7 @@ public: private: static void read_extra_data(JavaThread* current, const char* filename) NOT_CDS_RETURN; + static bool write_static_archive(ArchiveBuilder* builder, FileMapInfo* map_info, ArchiveHeapInfo* heap_info); static FileMapInfo* open_static_archive(); static FileMapInfo* open_dynamic_archive(); // use_requested_addr: If true (default), attempt to map at the address the diff --git a/src/hotspot/share/runtime/threads.cpp b/src/hotspot/share/runtime/threads.cpp index 9800f85dfe4..ea18ff3a006 100644 --- a/src/hotspot/share/runtime/threads.cpp +++ b/src/hotspot/share/runtime/threads.cpp @@ -818,7 +818,7 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) { #endif if (CDSConfig::is_dumping_static_archive()) { - MetaspaceShared::preload_and_dump(); + MetaspaceShared::preload_and_dump(CHECK_JNI_ERR); } if (log_is_enabled(Info, perf, class, link)) { diff --git a/test/hotspot/jtreg/runtime/cds/StaticWritingError.java b/test/hotspot/jtreg/runtime/cds/StaticWritingError.java new file mode 100644 index 00000000000..1bb3e7e721d --- /dev/null +++ b/test/hotspot/jtreg/runtime/cds/StaticWritingError.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8306580 + * @summary Test the writing error when archive file cannot be created + * @requires vm.cds + * @library /test/lib + * @run driver StaticWritingError + */ + +import java.io.File; +import jdk.test.lib.cds.CDSOptions; +import jdk.test.lib.cds.CDSTestUtils; +import jdk.test.lib.process.OutputAnalyzer; + +public class StaticWritingError { + public static void main(String[] args) throws Exception { + String directoryName = "nosuchdir"; + String archiveName = "staticWritingError.jsa"; + + // Perform static dump and attempt to write archive in unwritable directory + CDSOptions opts = (new CDSOptions()) + .addPrefix("-Xlog:cds") + .setArchiveName(directoryName + File.separator + archiveName); + OutputAnalyzer out = CDSTestUtils.createArchive(opts); + out.shouldHaveExitValue(1); + out.shouldContain("Unable to create shared archive file"); + out.shouldContain("Encountered error while dumping"); + } +}