8371944: AOT configuration is corrupted when app closes System.out

Reviewed-by: kvn, iveresov

Author: iklam

src/hotspot/share/cds/aotMetaspace.cpp

@@ -114,6 +114,7 @@ intx AOTMetaspace::_relocation_delta;
 char* AOTMetaspace::_requested_base_address;
 Array<Method*>* 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 @@ class VM_PopulateDumpSharedSpace : public VM_Operation {
 
 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;
 }

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<Method*>* _archived_method_handle_intrinsics;
+  static FileMapInfo* _output_mapinfo;
 
  public:
   enum {
@@ -185,6 +186,7 @@ class AOTMetaspace : AllStatic {
 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,

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;

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();

src/hotspot/share/cds/filemap.hpp

@@ -365,6 +365,7 @@ class FileMapInfo : public CHeapObj<mtInternal> {
   // 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);

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");
+        }
+    }
+}