Bug ID: JDK-8251438 Issues with our POSIX set_signal

Versions (Unresolved/Resolved/Fixed)

The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.

JDK 17
17 b03Fixed

Here are the issues with current os::Bsd::set_signal_handler()

#1 According to https://pubs.opengroup.org/onlinepubs/009695399/functions/sigaction.html "The storage occupied by sa_handler and sa_sigaction may overlap, and a conforming application shall not use both simultaneously.", however, our code assumes they use different storages, which just happens to work on BSD/Linux right now

#2 The "bool set_installed" in the API is unused and can/should be removed

#3 We should be reusing existing methods, such as "set_our_sigflags()" instead of repeating the same code

#4 Comments could use cleanup and be expanded for those not familiar with details of signals

#5 Not directly related, but we could add assert to check whether we re-set the same signal

Changeset: 70183f4d Author: Gerard Ziemski <gziemski@openjdk.org> Date: 2020-12-16 17:09:45 +0000 URL: https://git.openjdk.java.net/jdk/commit/70183f4d
16-12-2020
Wrapping the code in a static method like: // Implementation may use the same storage for both the sa_sigaction field and the sa_handler field, // so check for "sigAct.sa_flags == SA_SIGINFO" static void* get_signal_handler(struct sigaction action) { bool siginfo_flag_set = (action.sa_flags & SA_SIGINFO) != 0; if (siginfo_flag_set) { return CAST_FROM_FN_PTR(void, action.sa_sigaction); } else { return CAST_FROM_FN_PTR(void, action.sa_handler); } } will let us add a comment explaining why we do it this way. Also, there are a few places in the file that need this fix, so writing a single line code will let it be both concise and consistent.
16-11-2020
It looks like most (all?) mac/Linux/AIX signal related code can be refactored into common code - see JDK-8252324 "Signal related code should be shared among POSIX platforms"
25-08-2020
#1 is not limited to BSD, the code was copied from the Linux port and also exists in AIX port. We should simply be checking the SA_SIGINFO bit: void* oldhand = ((oldAct.sa_flags & SA_SIGINFO) != 0) ? ? CAST_FROM_FN_PTR(void, oldAct.sa_sigaction) : CAST_FROM_FN_PTR(void, oldAct.sa_handler); #2 is also not limited to BSD - we never pass "false" for set_installed. Can this be factored out to a common os_posix.cpp implementation?
12-08-2020
Re #1, instead of: void* oldhand = oldAct.sa_sigaction ? CAST_FROM_FN_PTR(void, oldAct.sa_sigaction) : CAST_FROM_FN_PTR(void, oldAct.sa_handler); we should have: void* oldhand = os::Bsd::get_signal_handler(oldAct); where: void* os::Bsd::get_signal_handler(struct sigaction action) { bool siginfo_flag_set = (action.sa_flags & SA_SIGINFO) != 0; if (siginfo_flag_set) { return CAST_FROM_FN_PTR(void, action.sa_sigaction); } else { return CAST_FROM_FN_PTR(void, action.sa_handler); } }
11-08-2020

Blocks :	JDK-8252324 - Signal related code should be shared among POSIX platforms
Relates :	JDK-8250637 - UseOSErrorReporting times out (on Mac and Linux)