8360164: AOT cache creation crashes in ~ThreadTotalCPUTimeClosure() #26008

iklam · 2025-06-26T23:41:40Z

java -XX:AOTMode=create calls vm_exit(0) to terminate the JVM (see this e-mail thread for the reason for doing so). However, at this point, the JVM has executed quite a lot of Java code and there are many threads in the JVM that would require a proper shutdown. See comments by @kimbarrett for a detailed analysis.

This fix calls JVM_Halt(0) instead of vm_exit(0) so that the proper shutdown code is executed.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8360164: AOT cache creation crashes in ~ThreadTotalCPUTimeClosure() (Bug - P3)

Reviewers

Vladimir Kozlov (@vnkozlov - Reviewer) Review applies to deed0c9d
David Holmes (@dholmes-ora - Reviewer) Review applies to f8222e38
Calvin Cheung (@calvinccheung - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26008/head:pull/26008
$ git checkout pull/26008

Update a local copy of the PR:
$ git checkout pull/26008
$ git pull https://git.openjdk.org/jdk.git pull/26008/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26008

View PR using the GUI difftool:
$ git pr show -t 26008

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26008.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-06-26T23:42:11Z

👋 Welcome back iklam! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-06-26T23:42:57Z

@iklam This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8360164: AOT cache creation crashes in ~ThreadTotalCPUTimeClosure()

Reviewed-by: ccheung, kvn, dholmes

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 53 new commits pushed to the master branch:

13a3927: 8359870: JVM crashes in AccessInternal::PostRuntimeDispatch
282ee40: 8359366: RunThese30M.java EXCEPTION_ACCESS_VIOLATION in JvmtiBreakpoints::clearall_in_class_at_safepoint
e7a4500: 8359707: Add classfile modification code to RedefineClassHelper
... and 50 more: https://git.openjdk.org/jdk/compare/cf75f1f9c6d2bc70c7133cb81c73a0ce0946dff9...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-06-26T23:43:26Z

@iklam The following label will be automatically applied to this pull request:

hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-06-26T23:48:06Z

Webrevs

vnkozlov

Good.

dholmes-ora · 2025-06-27T04:40:51Z

src/hotspot/share/cds/metaspaceShared.cpp

-        vm_exit(0);
      }
+      ThreadToNativeFromVM ttnfv(THREAD);
+      JVM_Halt(0);


JVM_Halt is the native backend for the JDK's System.halt() method. Whilst it might do what you need under-the-covers, I think it would be clearer to not actually call it, but just call before_exit and vm_exit directly. Even then you need to be careful as to how the halt parameter will be interpreted - in particular the affect on Jfr::on_vm_shutdown. In fact before_exit needs detailed examination to see if everything it does truly makes sense in this context.

I changed the code to call vm_direct_exit() instead. At this point, we are still in the middle of JNI_CreateJavaVM_inner

0 vm_direct_exit 1 MetaspaceShared::preload_and_dump 2 Threads::create_vm 3 JNI_CreateJavaVM_inner 4 JNI_CreateJavaVM 5 start_thread 6 clone3

so we may not have finished all the initialization that before_exit() may rely on.

Here's the code that's not yet executed:

jdk/src/hotspot/share/prims/jni.cpp

Lines 3591 to 3635 in da7080f

JavaThread *thread = JavaThread::current();

assert(!thread->has_pending_exception(), "should have returned not OK");

// thread is thread_in_vm here

*vm = (JavaVM *)(&main_vm);

*(JNIEnv**)penv = thread->jni_environment();

// mark creation complete for other JNI ops

Atomic::release_store(&vm_created, COMPLETE);

#if INCLUDE_JVMCI

if (EnableJVMCI) {

if (UseJVMCICompiler) {

// JVMCI is initialized on a CompilerThread

if (BootstrapJVMCI) {

JavaThread* THREAD = thread; // For exception macros.

JVMCICompiler* compiler = JVMCICompiler::instance(true, CATCH);

compiler->bootstrap(THREAD);

if (HAS_PENDING_EXCEPTION) {

HandleMark hm(THREAD);

vm_exit_during_initialization(Handle(THREAD, PENDING_EXCEPTION));

}

}

}

}

#endif

// Notify JVMTI

if (JvmtiExport::should_post_thread_life()) {

JvmtiExport::post_thread_start(thread);

}

JFR_ONLY(Jfr::on_thread_start(thread);)

if (ReplayCompiles) ciReplay::replay(thread);

#ifdef ASSERT

// Some platforms (like Win*) need a wrapper around these test

// functions in order to properly handle error conditions.

if (ErrorHandlerTest != 0) {

VMError::controlled_crash(ErrorHandlerTest);

}

#endif

// Since this is not a JVM_ENTRY we have to set the thread state manually before leaving.

ThreadStateTransition::transition_from_vm(thread, _thread_in_native);

MACOS_AARCH64_ONLY(thread->enable_wx(WXExec));

To be honest, I am not sure where is the starting point where it's safe to call before_exit() or System.exit(). At this point a lot of Java code has been executed (for setting up the module graph, etc), but I am not sure what happens when some of that Java code calls System.exit(), or whether such a scenario has been (sufficiently) tested.

BTW, JFR is disabled when we are dumping CDS:

jdk/src/hotspot/share/jfr/recorder/jfrRecorder.cpp

Lines 198 to 206 in da7080f

static bool is_cds_dump_requested() {

// we will not be able to launch recordings on startup if a cds dump is being requested

if (CDSConfig::is_dumping_archive() && JfrOptionSet::start_flight_recording_options() != nullptr) {

warning("JFR will be disabled during CDS dumping");

teardown_startup_support();

return true;

}

return false;

}

I am not sure what happens when some of that Java code calls System.exit(), or whether such a scenario has been (sufficiently) tested.

Initialization Java code should not be calling System.exit - ever. If something goes wrong it should throw an exception which is seen by the main thread doing the VM init and that in turn will lead to vm_exit_during_initialization.

To me the simplest model to use here would be to act as-if AOT creation was done by a small Java class such that you know the VM is fully initialized when you create the cache and then you can do a normal Java-level termination afterwards.

I tried doing that, but it's not straight-forward. At this point, some internal VM states have been zero-ed out by the AOT cache dumping code. As a result, it's no longer possible to resolve a new class. We get a crash when System.exit() resolves the Logger class:

[....] V [libjvm.so+0xf27aa2] InterpreterRuntime::resolve_from_cache(JavaThread*, Bytecodes::Code)+0x116 (interpreterRuntime.cpp:1001) j java.lang.System.getLogger(Ljava/lang/String;)Ljava/lang/System$Logger;+28 java.base@26-internal j java.lang.Shutdown.logRuntimeExit(I)V+2 java.base@26-internal j java.lang.Shutdown.exit(I)V+1 java.base@26-internal j java.lang.Runtime.exit(I)V+1 java.base@26-internal j java.lang.System.exit(I)V+4 java.base@26-internal v ~StubRoutines::call_stub 0x00007f392ae6072e

Since we need to backport this fix to JDK 25, I think we should go with the vm_direct_exit() now, and perhaps fix the AOT dumping code to avoid zeroing out VM states in a follow-up RFE.

dholmes-ora · 2025-06-27T04:41:50Z

src/hotspot/share/runtime/java.cpp

  // run Java code.
-  DynamicArchive::dump_at_exit(thread);
-  assert(!thread->has_pending_exception(), "must be");
+  if (CDSConfig::is_dumping_dynamic_archive()) {


I assume requesting a dynamic dump is mutually exclusive to the use of -XX:AOTMode=create?

…eads::create_vm() yet, so we may not have done all the initialization to be able to call before_exit()

dholmes-ora

Okay so the proposal now is that instead of trying to do a more "refined" termination process by calling before_exit etc (which is problematic because we are only partly initialized), we instead do a more abrupt termination by using vm_direct_exit.

This seems "safe" with regards to the issues Kim was commenting on.

calvinccheung

LGTM

iklam · 2025-07-01T20:21:34Z

Thanks @vnkozlov @calvinccheung @dholmes-ora for the review
/integrate

openjdk · 2025-07-01T20:22:13Z

Going to push as commit 7d7e60c.
Since your change was applied there have been 55 commits pushed to the master branch:

534d2b3: 8357064: cds/appcds/ArchiveRelocationTest.java failed with missing expected output
e9a62d7: 8361125: Fix typo in onTradAbsence
13a3927: 8359870: JVM crashes in AccessInternal::PostRuntimeDispatch
... and 52 more: https://git.openjdk.org/jdk/compare/cf75f1f9c6d2bc70c7133cb81c73a0ce0946dff9...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-07-01T20:22:22Z

@iklam Pushed as commit 7d7e60c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

iklam added 2 commits June 26, 2025 16:02

8360164: AOT cache creation crashes in ~ThreadTotalCPUTimeClosure()

0766b82

Use JVM_Halt instead

deed0c9

openjdk bot added the rfr Pull request is ready for review label Jun 26, 2025

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Jun 26, 2025

vnkozlov approved these changes Jun 26, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 26, 2025

dholmes-ora suggested changes Jun 27, 2025

View reviewed changes

Reverted to calling before_exit() directly

e0e5e30

openjdk bot removed the ready Pull request is ready to be integrated label Jun 27, 2025

Use vm_direct_exit instead, as at this point we have not finished Thr…

f8222e3

…eads::create_vm() yet, so we may not have done all the initialization to be able to call before_exit()

dholmes-ora approved these changes Jun 30, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 30, 2025

reverted change in before_exit() as it is no longer needed

b8f7452

openjdk bot removed the ready Pull request is ready to be integrated label Jul 1, 2025

calvinccheung approved these changes Jul 1, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 1, 2025

openjdk bot added the integrated Pull request has been integrated label Jul 1, 2025

openjdk bot closed this Jul 1, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jul 1, 2025

	JavaThread *thread = JavaThread::current();
	assert(!thread->has_pending_exception(), "should have returned not OK");
	// thread is thread_in_vm here
	vm = (JavaVM )(&main_vm);
	(JNIEnv*)penv = thread->jni_environment();
	// mark creation complete for other JNI ops
	Atomic::release_store(&vm_created, COMPLETE);

	#if INCLUDE_JVMCI
	if (EnableJVMCI) {
	if (UseJVMCICompiler) {
	// JVMCI is initialized on a CompilerThread
	if (BootstrapJVMCI) {
	JavaThread* THREAD = thread; // For exception macros.
	JVMCICompiler* compiler = JVMCICompiler::instance(true, CATCH);
	compiler->bootstrap(THREAD);
	if (HAS_PENDING_EXCEPTION) {
	HandleMark hm(THREAD);
	vm_exit_during_initialization(Handle(THREAD, PENDING_EXCEPTION));
	}
	}
	}
	}
	#endif

	// Notify JVMTI
	if (JvmtiExport::should_post_thread_life()) {
	JvmtiExport::post_thread_start(thread);
	}

	JFR_ONLY(Jfr::on_thread_start(thread);)

	if (ReplayCompiles) ciReplay::replay(thread);

	#ifdef ASSERT
	// Some platforms (like Win*) need a wrapper around these test
	// functions in order to properly handle error conditions.
	if (ErrorHandlerTest != 0) {
	VMError::controlled_crash(ErrorHandlerTest);
	}
	#endif

	// Since this is not a JVM_ENTRY we have to set the thread state manually before leaving.
	ThreadStateTransition::transition_from_vm(thread, _thread_in_native);
	MACOS_AARCH64_ONLY(thread->enable_wx(WXExec));

	static bool is_cds_dump_requested() {
	// we will not be able to launch recordings on startup if a cds dump is being requested
	if (CDSConfig::is_dumping_archive() && JfrOptionSet::start_flight_recording_options() != nullptr) {
	warning("JFR will be disabled during CDS dumping");
	teardown_startup_support();
	return true;
	}
	return false;
	}

8360164: AOT cache creation crashes in ~ThreadTotalCPUTimeClosure() #26008

8360164: AOT cache creation crashes in ~ThreadTotalCPUTimeClosure() #26008

Uh oh!

Conversation

iklam commented Jun 26, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jun 26, 2025

Uh oh!

openjdk bot commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jun 26, 2025

Uh oh!

mlbridge bot commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

vnkozlov left a comment

Choose a reason for hiding this comment

Uh oh!

dholmes-ora Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

iklam Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

dholmes-ora Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

iklam Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

dholmes-ora Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

iklam Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

dholmes-ora left a comment

Choose a reason for hiding this comment

Uh oh!

calvinccheung left a comment

Choose a reason for hiding this comment

Uh oh!

iklam commented Jul 1, 2025

Uh oh!

openjdk bot commented Jul 1, 2025

Uh oh!

openjdk bot commented Jul 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

iklam commented Jun 26, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jun 26, 2025 •

edited

Loading

mlbridge bot commented Jun 26, 2025 •

edited

Loading