RFR: 8291569: Consider removing JNI checks for signals SIGPIPE and SIGXFSZ [v2]
Thomas Stuefe
stuefe at openjdk.org
Tue Jan 24 12:46:04 UTC 2023
On Mon, 23 Jan 2023 00:37:01 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Simple enhancement to skip checking of the handlers for SIGPIPE and SIGXFSZ as we really don't care if they change, and we expect that they can.
>>
>> The signal checking code might seem to have a redundancy as we skip these signals in two places, but the primary use of the `do_check_signal_periodically` array is to allow for only issuing one warning per signal. I could have skipped them in either place and get the same effect but it seemed inappropriate to set the array entry and not do the check; and vice-versa.
>>
>> Testing: just basic sanity test tiers 1-3. This doesn't affect any application behaviour.
>>
>> There are no regression tests in this specific area and it did not seem worth the effort creating one just for this.
>>
>> Thanks.
>
> David Holmes has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
>
> - Merge branch 'master' into 8291569-signals
> - 8291569: Consider removing JNI checks for signals SIGPIPE and SIGXFSZ
Oops, I missed the discussion completely. My take on this:
We need to ignore SIGPIPE and SIGXFSZ because the default action for them is to terminate the process, which we usually don't want (btw we don't do this for SIGLOST nor SIGXCPU, which seems inconsistent). By handling those signals, we cause the associated system functions to return errors instead (eg EPIPE).
IIUC we normally expect user code to clearly communicate to us when they use their own signal handler. Via signal chaining (UseSignalChaining, AllowUserSignalHandlers etc). But setting a SIGPIPE handler - or temporarily disabling it - without signal chaining seems to be such a common pattern that we now remove the warning instead. We basically throw the hands in the air and give up.
Have I read the intentions of this patch wrong?
-------------
PR: https://git.openjdk.org/jdk/pull/12062
More information about the hotspot-runtime-dev
mailing list