JDK-8293493: Signal Handlers printout should show signal block state #10199

tstuefe · 2022-09-07T14:51:27Z

I needed this recently. We print out the signal block mask, which is fine, but more interesting would be the existing block state at the point of the crash/assertion.

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-8293493: Signal Handlers printout should show signal block state

Reviewers

David Holmes (@dholmes-ora - Reviewer)
Johan Sjölen (@jdksjolen - Author)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10199

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

Using diff file

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

bridgekeeper · 2022-09-07T14:53:43Z

👋 Welcome back stuefe! 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 · 2022-09-07T14:57:01Z

@tstuefe 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.

…ghandler-printout

mlbridge · 2022-09-07T17:03:25Z

Webrevs

00: Full (3c5eeb45)

tstuefe · 2022-09-14T12:59:22Z

Friendly ping. Change is really simple.

dholmes-ora

This seems okay though may be of limited use if the crash is from a signal as IIRC that in itself will result in various signals getting blocked whilst we remain in the signal-handler context.

Thanks.

dholmes-ora · 2022-09-19T02:31:48Z

src/hotspot/os/posix/signals_posix.cpp

@@ -1414,6 +1413,11 @@ void PosixSignals::print_signal_handler(outputStream* st, int sig,
  sigaction(sig, NULL, &current_act);

  print_single_signal_handler(st, &current_act, buf, buflen);
+
+  sigset_t thread_sig_mask;
+  if (::pthread_sigmask(/* ignored */ SIG_BLOCK, NULL, &thread_sig_mask) == 0) {


Nit: no need for :: scoping (there is one other existing unnecessary use as well you could remove :) ).

openjdk · 2022-09-19T02:35:01Z

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

8293493: Signal Handlers printout should show signal block state

Reviewed-by: dholmes, jsjolen

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

cd1cdcd: 8293116: Incremental JDK build could be sped up
e9401e6: 8293364: IGV: Refactor Action in EditorTopComponent and fix minor bugs
844a95b: 8292892: Javadoc index descriptions are not deterministic
8d1dd6a: 8294076: Improve ant detection in idea.sh
4e7cb15: 8293480: IGV: Update Bytecode and ControlFlow Component immediately when opening a new graph
8ecdaa6: 8294000: Filler array klass should be in jdk/vm/internal, not in java/vm/internal
379f309: 8287217: C2: PhaseCCP: remove not visited nodes, prevent type inconsistency
12e3510: 8293798: Fix test bugs due to incompatibility with -XX:+AlwaysIncrementalInline
cb72f80: 8293978: Duplicate simple loop back-edge will crash the vm
cddd6de: 8279941: sun/security/pkcs11/Signature/TestDSAKeyLength.java fails when NSS version detection fails
... and 145 more: https://git.openjdk.org/jdk/compare/1ee59adcfead2128316556121c1711d308c7ea01...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.

tstuefe · 2022-09-19T05:34:06Z

This seems okay though may be of limited use if the crash is from a signal as IIRC that in itself will result in various signals getting blocked whilst we remain in the signal-handler context.

True, but its interesting to know if we block what we think we block :-)

jdksjolen · 2022-09-21T09:01:39Z

This LGTM. It fails tier1 tests because it doesn't include the commit that problem lists runtime/os/TestTracePageSizes.java on linux-x64. Merging to master should fix that issue.

tstuefe · 2022-09-21T10:54:53Z

This LGTM. It fails tier1 tests because it doesn't include the commit that problem lists runtime/os/TestTracePageSizes.java on linux-x64. Merging to master should fix that issue.

Thanks @jdksjolen ! Mind hitting the approve button?

tstuefe · 2022-09-21T12:05:48Z

/integrate

openjdk · 2022-09-21T12:07:06Z

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

da4fdfb: 8293659: Improve UnsatisfiedLinkError error message to include dlopen error details
cd1cdcd: 8293116: Incremental JDK build could be sped up
e9401e6: 8293364: IGV: Refactor Action in EditorTopComponent and fix minor bugs
844a95b: 8292892: Javadoc index descriptions are not deterministic
8d1dd6a: 8294076: Improve ant detection in idea.sh
4e7cb15: 8293480: IGV: Update Bytecode and ControlFlow Component immediately when opening a new graph
8ecdaa6: 8294000: Filler array klass should be in jdk/vm/internal, not in java/vm/internal
379f309: 8287217: C2: PhaseCCP: remove not visited nodes, prevent type inconsistency
12e3510: 8293798: Fix test bugs due to incompatibility with -XX:+AlwaysIncrementalInline
cb72f80: 8293978: Duplicate simple loop back-edge will crash the vm
... and 146 more: https://git.openjdk.org/jdk/compare/1ee59adcfead2128316556121c1711d308c7ea01...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-09-21T12:07:30Z

@tstuefe Pushed as commit d14e96d.

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

tstuefe added 2 commits September 7, 2022 16:50

JDK-8293493-print-signal-block-state-in-sighandler-printout

4077097

dont scream so much

1406b05

tstuefe changed the title ~~JDK-8293493: Signal Handlers printout in hs-err file should tell which signals are blocked~~ JDK-8293493: Signal Handlers printout should show signal block state Sep 7, 2022

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Sep 7, 2022

Merge branch 'master' into JDK-8293493-print-signal-block-state-in-si…

3c5eeb4

…ghandler-printout

tstuefe marked this pull request as ready for review September 7, 2022 16:57

openjdk bot added the rfr Pull request is ready for review label Sep 7, 2022

dholmes-ora approved these changes Sep 19, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 19, 2022

jdksjolen approved these changes Sep 21, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Sep 21, 2022

openjdk bot closed this Sep 21, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8293493: Signal Handlers printout should show signal block state #10199

JDK-8293493: Signal Handlers printout should show signal block state #10199

tstuefe commented Sep 7, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Sep 7, 2022

openjdk bot commented Sep 7, 2022

mlbridge bot commented Sep 7, 2022

tstuefe commented Sep 14, 2022

dholmes-ora left a comment

dholmes-ora Sep 19, 2022

openjdk bot commented Sep 19, 2022 •

edited

Loading

tstuefe commented Sep 19, 2022

jdksjolen commented Sep 21, 2022

tstuefe commented Sep 21, 2022

tstuefe commented Sep 21, 2022

openjdk bot commented Sep 21, 2022

openjdk bot commented Sep 21, 2022

JDK-8293493: Signal Handlers printout should show signal block state #10199

JDK-8293493: Signal Handlers printout should show signal block state #10199

Conversation

tstuefe commented Sep 7, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Sep 7, 2022

openjdk bot commented Sep 7, 2022

mlbridge bot commented Sep 7, 2022

Webrevs

tstuefe commented Sep 14, 2022

dholmes-ora left a comment

Choose a reason for hiding this comment

dholmes-ora Sep 19, 2022

Choose a reason for hiding this comment

openjdk bot commented Sep 19, 2022 • edited Loading

tstuefe commented Sep 19, 2022

jdksjolen commented Sep 21, 2022

tstuefe commented Sep 21, 2022

tstuefe commented Sep 21, 2022

openjdk bot commented Sep 21, 2022

openjdk bot commented Sep 21, 2022

tstuefe commented Sep 7, 2022 •

edited by openjdk bot

Loading

openjdk bot commented Sep 19, 2022 •

edited

Loading