diff --git a/src/hotspot/share/cds/aotMetaspace.cpp b/src/hotspot/share/cds/aotMetaspace.cpp index 8642b1a6de8..42d41e6ae89 100644 --- a/src/hotspot/share/cds/aotMetaspace.cpp +++ b/src/hotspot/share/cds/aotMetaspace.cpp @@ -114,6 +114,7 @@ intx AOTMetaspace::_relocation_delta; char* AOTMetaspace::_requested_base_address; Array* AOTMetaspace::_archived_method_handle_intrinsics = nullptr; bool AOTMetaspace::_use_optimized_module_handling = true; +FileMapInfo* AOTMetaspace::_output_mapinfo = nullptr; // The CDS archive is divided into the following regions: // rw - read-write metadata @@ -322,6 +323,24 @@ void AOTMetaspace::initialize_for_static_dump() { AOTMetaspace::unrecoverable_writing_error(); } _symbol_region.init(&_symbol_rs, &_symbol_vs); + if (CDSConfig::is_dumping_preimage_static_archive()) { + // We are in the AOT training run. User code is executed. + // + // On Windows, if the user code closes System.out and we open the AOT config file for output + // only at VM exit, we might get back the same file HANDLE as stdout, and the AOT config + // file may get corrupted by UL logs. By opening early, we ensure that the output + // HANDLE is different than stdout so we can avoid such corruption. + open_output_mapinfo(); + } else { + // No need for the above as we won't execute any user code. + } +} + +void AOTMetaspace::open_output_mapinfo() { + const char* static_archive = CDSConfig::output_archive_path(); + assert(static_archive != nullptr, "sanity"); + _output_mapinfo = new FileMapInfo(static_archive, true); + _output_mapinfo->open_as_output(); } // Called by universe_post_init() @@ -655,15 +674,14 @@ private: public: - VM_PopulateDumpSharedSpace(StaticArchiveBuilder& b) : - VM_Operation(), _mapped_heap_info(), _streamed_heap_info(), _map_info(nullptr), _builder(b) {} + VM_PopulateDumpSharedSpace(StaticArchiveBuilder& b, FileMapInfo* map_info) : + VM_Operation(), _mapped_heap_info(), _streamed_heap_info(), _map_info(map_info), _builder(b) {} bool skip_operation() const { return false; } VMOp_Type type() const { return VMOp_PopulateDumpSharedSpace; } ArchiveMappedHeapInfo* mapped_heap_info() { return &_mapped_heap_info; } ArchiveStreamedHeapInfo* streamed_heap_info() { return &_streamed_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 @@ -795,12 +813,6 @@ void VM_PopulateDumpSharedSpace::doit() { CppVtables::zero_archived_vtables(); // Write the archive file - if (CDSConfig::is_dumping_final_static_archive()) { - FileMapInfo::free_current_info(); // FIXME: should not free current info - } - const char* static_archive = CDSConfig::output_archive_path(); - assert(static_archive != nullptr, "sanity"); - _map_info = new FileMapInfo(static_archive, true); _map_info->populate_header(AOTMetaspace::core_region_alignment()); _map_info->set_early_serialized_data(early_serialized_data); _map_info->set_serialized_data(serialized_data); @@ -1138,7 +1150,14 @@ void AOTMetaspace::dump_static_archive_impl(StaticArchiveBuilder& builder, TRAPS } #endif - VM_PopulateDumpSharedSpace op(builder); + if (!CDSConfig::is_dumping_preimage_static_archive()) { + if (CDSConfig::is_dumping_final_static_archive()) { + FileMapInfo::free_current_info(); // FIXME: should not free current info + } + open_output_mapinfo(); + } + + VM_PopulateDumpSharedSpace op(builder, _output_mapinfo); VMThread::execute(&op); if (AOTCodeCache::is_on_for_dump() && CDSConfig::is_dumping_final_static_archive()) { @@ -1152,7 +1171,9 @@ void AOTMetaspace::dump_static_archive_impl(StaticArchiveBuilder& builder, TRAPS CDSConfig::disable_dumping_aot_code(); } - bool status = write_static_archive(&builder, op.map_info(), op.mapped_heap_info(), op.streamed_heap_info()); + bool status = write_static_archive(&builder, _output_mapinfo, op.mapped_heap_info(), op.streamed_heap_info()); + assert(!_output_mapinfo->is_open(), "Must be closed already"); + _output_mapinfo = nullptr; if (status && CDSConfig::is_dumping_preimage_static_archive()) { tty->print_cr("%s AOTConfiguration recorded: %s", CDSConfig::has_temp_aot_config_file() ? "Temporary" : "", AOTConfiguration); @@ -1173,11 +1194,10 @@ bool AOTMetaspace::write_static_archive(ArchiveBuilder* builder, // relocate the data so that it can be mapped to AOTMetaspace::requested_base_address() // without runtime relocation. builder->relocate_to_requested(); - - map_info->open_as_output(); if (!map_info->is_open()) { return false; } + map_info->prepare_for_writing(); builder->write_archive(map_info, mapped_heap_info, streamed_heap_info); return true; } diff --git a/src/hotspot/share/cds/aotMetaspace.hpp b/src/hotspot/share/cds/aotMetaspace.hpp index bfd9f4bcc75..1712a7865ad 100644 --- a/src/hotspot/share/cds/aotMetaspace.hpp +++ b/src/hotspot/share/cds/aotMetaspace.hpp @@ -60,6 +60,7 @@ class AOTMetaspace : AllStatic { static char* _requested_base_address; static bool _use_optimized_module_handling; static Array* _archived_method_handle_intrinsics; + static FileMapInfo* _output_mapinfo; public: enum { @@ -185,6 +186,7 @@ public: private: static void read_extra_data(JavaThread* current, const char* filename) NOT_CDS_RETURN; static void fork_and_dump_final_static_archive(TRAPS); + static void open_output_mapinfo(); static bool write_static_archive(ArchiveBuilder* builder, FileMapInfo* map_info, ArchiveMappedHeapInfo* mapped_heap_info, diff --git a/src/hotspot/share/cds/dynamicArchive.cpp b/src/hotspot/share/cds/dynamicArchive.cpp index 85e59e23f8c..8fae8dabf8c 100644 --- a/src/hotspot/share/cds/dynamicArchive.cpp +++ b/src/hotspot/share/cds/dynamicArchive.cpp @@ -353,6 +353,7 @@ void DynamicArchiveBuilder::write_archive(char* serialized_data, AOTClassLocatio assert(dynamic_info != nullptr, "Sanity"); dynamic_info->open_as_output(); + dynamic_info->prepare_for_writing(); ArchiveBuilder::write_archive(dynamic_info, nullptr, nullptr); address base = _requested_dynamic_archive_bottom; diff --git a/src/hotspot/share/cds/filemap.cpp b/src/hotspot/share/cds/filemap.cpp index 61df0a86b41..0eeb96bb269 100644 --- a/src/hotspot/share/cds/filemap.cpp +++ b/src/hotspot/share/cds/filemap.cpp @@ -779,7 +779,9 @@ void FileMapInfo::open_as_output() { } _fd = fd; _file_open = true; +} +void FileMapInfo::prepare_for_writing() { // Seek past the header. We will write the header after all regions are written // and their CRCs computed. size_t header_bytes = header()->header_size(); diff --git a/src/hotspot/share/cds/filemap.hpp b/src/hotspot/share/cds/filemap.hpp index 2a761843e47..fbd3c8e1681 100644 --- a/src/hotspot/share/cds/filemap.hpp +++ b/src/hotspot/share/cds/filemap.hpp @@ -365,6 +365,7 @@ public: // File manipulation. bool open_as_input() NOT_CDS_RETURN_(false); void open_as_output(); + void prepare_for_writing(); void write_header(); void write_region(int region, char* base, size_t size, bool read_only, bool allow_exec); diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotCache/CloseSystemOut.java b/test/hotspot/jtreg/runtime/cds/appcds/aotCache/CloseSystemOut.java new file mode 100644 index 00000000000..1f4111ecfb1 --- /dev/null +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotCache/CloseSystemOut.java @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2025, 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 + * @summary AOT configuration should not be corrupted even if the app closes System.out in the training run + * @bug 8371944 + * @library /test/jdk/lib/testlibrary /test/lib + * @build CloseSystemOut + * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar CloseSystemOutApp + * @run driver CloseSystemOut + */ + +import java.io.PrintWriter; +import jdk.test.lib.cds.CDSAppTester; +import jdk.test.lib.helpers.ClassFileInstaller; +import jdk.test.lib.process.OutputAnalyzer; + +public class CloseSystemOut { + static final String appJar = ClassFileInstaller.getJarPath("app.jar"); + static final String mainClass = "CloseSystemOutApp"; + + public static void main(String[] args) throws Exception { + Tester tester = new Tester(); + tester.run(new String[] {"AOT", "--two-step-training"} ); + } + + static class Tester extends CDSAppTester { + public Tester() { + super(mainClass); + } + + @Override + public String classpath(RunMode runMode) { + return appJar; + } + + @Override + public String[] appCommandLine(RunMode runMode) { + return new String[] {mainClass}; + } + + @Override + public void checkExecution(OutputAnalyzer out, RunMode runMode) { + if (runMode != RunMode.ASSEMBLY) { + out.shouldContain("Hello Confused World"); + } + } + } +} + +class CloseSystemOutApp { + public static void main(String args[]) { + // Naive code that ends up closing System.out/err when we + // leave the "try" block + try (var err = new PrintWriter(System.err); + var out = new PrintWriter(System.out)) { + out.println("Hello Confused World"); + } + } +}