8330464: hserr generic events - add entry for the before_exit calls #18809

MBaesken · 2024-04-17T08:20:03Z

We currently miss a hs_err file generic event entry for before_exit calls. Those would be helpful to see clearly that a crash happened in a shutdown phase (we had this case recently and would have liked to have a hserr event log entry showing this more clearly).

Additionally while testing the event I called System.exit(0) to check the event.
This showed a native backtrace like this

Stack: [0x00007f1cfe825000,0x00007f1cfe926000], sp=0x00007f1cfe924530, free space=1021k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V [libjvm.so+0x8fb21b] before_exit(JavaThread*, bool)+0x4bb (java.cpp:376)
V [libjvm.so+0x9da27c] JVM_Halt+0x6c (jvm.cpp:442)
j java.lang.Shutdown.halt0(I)V+0 java.base@23.0.0.1-internal
j java.lang.Shutdown.halt(I)V+7 java.base@23.0.0.1-internal
j java.lang.Shutdown.exit(I)V+16 java.base@23.0.0.1-internal
j java.lang.Runtime.exit(I)V+14 java.base@23.0.0.1-internal
j java.lang.System.exit(I)V+4 java.base@23.0.0.1-internal

So the comment in thread.cpp seems to be outdated, Threads::destroy_vm() is not in the callstack and vm_exit() is not seen as well in the callstack so probably the comment needs adjustment.

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-8330464: hserr generic events - add entry for the before_exit calls (Enhancement - P4)

Reviewers

Richard Reingruber (@reinrich - Reviewer)
Lutz Schmidt (@RealLucy - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18809

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2024-04-17T08:20:45Z

👋 Welcome back mbaesken! 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 · 2024-04-17T08:21:05Z

@MBaesken 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:

8330464: hserr generic events - add entry for the before_exit calls

Reviewed-by: rrich, lucy

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 90 new commits pushed to the master branch:

88a5dce: 8330805: ARM32 build is broken after JDK-8139457
7157eea: 8327290: Remove unused notproduct option TraceInvocationCounterOverflow
b4cea70: 8330693: Generational ZGC: Simplify ZAddress::finalizable_good and ZAddress::mark_good
412e306: 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O
b07e153: 8330362: G1: Inline offset array element accessor in G1BlockOffsetTable
b6518a5: 8329417: Remove objects with no pointers from relocation bitmap
383fe6e: 8330388: Remove invokedynamic cache index encoding
2ea8926: 8330961: Remove redundant public specifier in ModRefBarrierSet
3d5eeac: 8289770: Remove Windows version macro from ShellFolder2.cpp
6158da5: 8330108: Increase CipherInputStream buffer size
... and 80 more: https://git.openjdk.org/jdk/compare/9d63fee49c3b365e19cf492412a6b6d8c9633964...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 · 2024-04-17T08:21:45Z

@MBaesken 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 · 2024-04-17T08:25:23Z

Webrevs

reinrich · 2024-04-18T07:10:09Z

src/hotspot/share/runtime/threads.cpp

@@ -833,7 +833,7 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {

 // Threads::destroy_vm() is normally called from jni_DestroyJavaVM() when
 // the program falls off the end of main(). Another VM exit path is through
-// vm_exit() when the program calls System.exit() to return a value or when
+// vm_exit() when


I think you have to read it like this:

Exit Path 1: reaching from Threads::destroy_vm() Exit Path 2: reaching from - vm_exit() - System.exit() - Serious error

Maybe add commas to disambiguate.
(https://en.wikipedia.org/wiki/Serial_comma)

I adjusted the comment and brought back the deleted part.

reinrich · 2024-04-18T07:53:37Z

src/hotspot/share/runtime/threads.cpp

+// vm_exit() when the program calls System.exit() to return a value, or when
+// there is a serious error in VM.


I'm not a native english speaker but I think all elements in the series need to be separated by commas.
Otherwise the misinterpretation that System.exit() calls vm_exit() is more likely.

Suggested change

// vm_exit() when the program calls System.exit() to return a value, or when

// there is a serious error in VM.

// vm_exit(), when the program calls System.exit() to return a value, or when

// there is a serious error in VM.

reinrich

Lgtm.

RealLucy

LGTM.

MBaesken · 2024-04-24T12:54:28Z

Thanks for the reviews !

/integrate

openjdk · 2024-04-24T12:55:36Z

Going to push as commit 45ed97f.
Since your change was applied there have been 108 commits pushed to the master branch:

e311ba3: 8330626: ZGC: Windows address space placeholders not managed correctly
e923dfe: 8314592: Add shortcut to SymbolLookup::find
1519081: 8330303: Crash: assert(_target_jt == nullptr || _target_jt->vthread() == target_h()) failed
2bb5cf5: 8330155: Serial: Remove TenuredSpace
7bb59dc: 8330670: Obsolete ScavengeBeforeFullGC
174d626: 8328741: serviceability/jvmti/ObjectMonitorUsage/ObjectMonitorUsage.java failed with unexpected owner
e681e9b: 8330819: C2 SuperWord: bad dominance after pre-loop limit adjustment with base that has CastLL after pre-loop
c439c8c: 8323429: Missing C2 optimization for FP min/max when both inputs are same
165ba87: 8330587: IGV: remove ControlFlowTopComponent
3ccb64c: 8318446: C2: optimize stores into primitive arrays by combining values into larger store
... and 98 more: https://git.openjdk.org/jdk/compare/9d63fee49c3b365e19cf492412a6b6d8c9633964...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-04-24T12:55:42Z

@MBaesken Pushed as commit 45ed97f.

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

JDK-8330464

e78af30

openjdk bot changed the title ~~JDK-8330464: hserr generic events - add entry for the before_exit calls~~ 8330464: hserr generic events - add entry for the before_exit calls Apr 17, 2024

openjdk bot added the rfr Pull request is ready for review label Apr 17, 2024

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Apr 17, 2024

reinrich reviewed Apr 18, 2024

View reviewed changes

adjust comment in threads.cpp

30a6560

reinrich reviewed Apr 18, 2024

View reviewed changes

improve comment

3d29c33

reinrich approved these changes Apr 19, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Apr 19, 2024

RealLucy approved these changes Apr 19, 2024

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Apr 24, 2024

openjdk bot closed this Apr 24, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8330464: hserr generic events - add entry for the before_exit calls #18809

8330464: hserr generic events - add entry for the before_exit calls #18809

MBaesken commented Apr 17, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Apr 17, 2024

openjdk bot commented Apr 17, 2024 •

edited

Loading

openjdk bot commented Apr 17, 2024

mlbridge bot commented Apr 17, 2024 •

edited

Loading

reinrich Apr 18, 2024

MBaesken Apr 18, 2024

reinrich Apr 18, 2024

reinrich left a comment

RealLucy left a comment

MBaesken commented Apr 24, 2024

openjdk bot commented Apr 24, 2024

openjdk bot commented Apr 24, 2024

		// vm_exit() when the program calls System.exit() to return a value, or when
		// there is a serious error in VM.

8330464: hserr generic events - add entry for the before_exit calls #18809

8330464: hserr generic events - add entry for the before_exit calls #18809

Conversation

MBaesken commented Apr 17, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Apr 17, 2024

openjdk bot commented Apr 17, 2024 • edited Loading

openjdk bot commented Apr 17, 2024

mlbridge bot commented Apr 17, 2024 • edited Loading

Webrevs

reinrich Apr 18, 2024

Choose a reason for hiding this comment

MBaesken Apr 18, 2024

Choose a reason for hiding this comment

reinrich Apr 18, 2024

Choose a reason for hiding this comment

reinrich left a comment

Choose a reason for hiding this comment

RealLucy left a comment

Choose a reason for hiding this comment

MBaesken commented Apr 24, 2024

openjdk bot commented Apr 24, 2024

openjdk bot commented Apr 24, 2024

MBaesken commented Apr 17, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Apr 17, 2024 •

edited

Loading

mlbridge bot commented Apr 17, 2024 •

edited

Loading