Fix signal handler registration in the JVM

[babsingh]

omrsig.cpp::omrsig_primary_sigaction/omrsig_primary_signal doesn't
support registration with SIG_DFL/SIG_IGN/NULL as the signal handler.

1. SIG_IGN is passed as the signal handler for SIGPIPE.
2. NULL is passed as the new signal handler for SIGILL.

In the above cases, omrsig_primary_sigaction/omrsig_primary_signal
shouldn't be used for registering a signal handler.

Now onwards, only <signal.h>::sigaction/signal will used in the above
cases.

SIGPIPE needs to be ignored for the Java Attach API to work properly.

Fixes #2871

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

[pdbain-ibm]

@pshipton would you kindly run an extended build so we can see it this fixes the attach API error handling failure?
@babsingh did you test it with the test case supplied with issue #2871?

Thank you.

[pshipton]

Are there any other places/signals with a similar problem?

jenkins test extended zlinux jdk8

[pshipton]

@babsingh does this place also need to be fixed?

https://github.com/eclipse/openj9/blob/4833764455a3a3a9c0873ffb2e353d8b121ba49e/runtime/vm/jvminit.c#L6490

[babsingh]

did you test it with the test case supplied with issue #2871?

yes, it passes.

OMRSIG_SIGACTION(SIGILL, NULL, &oldHandler);

yes, this place would also need to be fixed since omrsig_primary_sigaction would return error for NULL signal handler without setting the old handler. so, omrsig lib shouldn't be used in this scenario. i will update the PR.

[pshipton]

https://ci.eclipse.org/openj9/job/PullRequest-Extended-JDK8-linux_390-64_cmprssptrs-OpenJ9/72/

[babsingh]

@pshipton I have updated two other places where omrsig_primary_sigaction/omrsig_primary_signal shouldn't be used.

[pshipton]

The port library code wasn't affected by #2631 as I understand it. Plus the code just seems wrong. I think we should leave this one for further investigation and a different PR.

[pshipton]

Waiting for https://ci.eclipse.org/openj9/job/PullRequest-Extended-JDK8-linux_390-64_cmprssptrs-OpenJ9/72/ to complete and pass.

jenkins compile xlinux jdk8

[babsingh]

Removed changes related to SIG_RI_INTERRUPT from this PR.

Opened #2967 to resolve the issue related to SIG_RI_INTERRUPT.

[babsingh]

@pshipton https://ci.eclipse.org/openj9/job/PullRequest-Extended-JDK8-linux_390-64_cmprssptrs-OpenJ9/72/ has passed.

I have verified that org.openj9.test.attachAPI.TestAttachErrorHandling:test_startupShutdownRobustness is passing: https://ci.eclipse.org/openj9/job/PullRequest-Extended-JDK8-linux_390-64_cmprssptrs-OpenJ9/72/consoleFull.

[DanHeidinga]

I think we want to revisit this and the original PR #2631

The extra rules on the use of signal vs omr_* apis increases the likelihood of future bugs when these rules are missed by developers & reviewers.

[babsingh]

we want to revisit this

Agreed, we should revisit this. right approach: omrsig_primary_signal/omrsig_primary_sigaction should behave similar to signal/sigaction. omrsig_primary_signal/omrsig_primary_sigaction should be updated to handle SIG_IGN/SIG_DFL/NULL cases similar to signal/sigaction. this way omrsig_primary_signal/omrsig_primary_sigaction and signal/sigaction can be used interchangeably. @DanHeidinga do you agree? if agreed, i will open an OMR issue.

[pshipton]

omrsig_primary_signal/omrsig_primary_sigaction should behave similar to signal/sigaction

I'm not sure this actually works. There is a difference between user code calling sigaction, and the VM calling sigaction. If the VM has registered a signal we don't want user code to then set it to ignore or default.

[DanHeidinga]

@babsingh Can you open an issue and outline the changes that have been made so far, why they were made, and what you think the right path forward is?

Let's have the discussion in a top-level issue rather than in the comments on an already merged PR. Too easy to lose this discussion :)