-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8279878: java/awt/font/JNICheck/JNICheck.sh test fails on Ubuntu 21.10 #7091
Conversation
👋 Welcome back prr! A progress list of the required criteria for merging this PR into |
Maybe we can ask some ppl from the hotspot team about this issue, is it a critical thing that these handlers are overwritten by probably GTK3? Can it cause a crash or something? |
In hotspot, we want to explicitly ignore SIGPIPE and SIGXFSZ (see https://bugs.openjdk.java.net/browse/JDK-4229104 and https://bugs.openjdk.java.net/browse/JDK-6499219). But we don't do SIG_IGN, since we still need to give chained handlers the opportunity to handle them. See jdk/src/hotspot/os/posix/signals_posix.cpp Lines 618 to 623 in fef8f2d
So now, from the OS perspective, we handle SIGPIPE. We just don't do anything but return from the signal handler (ignoring chained handlers for now). If someone outside sets SIGPIPE to SIG_IGN, then we get subtle changes:
It's all pretty unexciting. Nothing would crash. There may be an argument for skipping the test on SIGPIPE if it is SIG_IGN and we don't have chained handlers. If it seems such a standard thing to do for third party libraries. Idk. Maybe open a separate bug? |
Probably we have some tests which may validate that even if our handlers are not default the external handler will call ours by chain(or something like that)? Or maybe we should restore them? |
The discussion seems to have gone off somewhere unrelated to this client test. |
I am still not sure can we do something about this, depending on that we should update the test/problem list/file another client bug/another hotspot bug? |
I can't work out what you are saying/expecting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So can we file a bug against someone? We should do something about it.
@prrace 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:
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 263 new commits pushed to the
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 |
/integrate |
Going to push as commit 2f48a3f.
Your commit was automatically rebased without conflicts. |
Some more signal handler related warning strings from the VM need to be excluded from what this test considers a failure
See the bug for more info.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7091/head:pull/7091
$ git checkout pull/7091
Update a local copy of the PR:
$ git checkout pull/7091
$ git pull https://git.openjdk.java.net/jdk pull/7091/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 7091
View PR using the GUI difftool:
$ git pr show -t 7091
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7091.diff