JDK-8295125 : os::signal should be os specific
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 20
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2022-10-11
  • Updated: 2022-10-31
  • Resolved: 2022-10-31
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 20
20 masterFixed
Related Reports
Relates :  
JDK-8294594 - Fix cast-function-type warnings in signal handling code
Relates :  
JDK-8295702 - Signal.handle can't correctly restore all signal handlers on Posix systems
Description
The second argument for os::signal is a void* handler function.

The posix implementation of os::signal expects that handler argument to be a sigaction handler, taking 3 arguments.

The Windows implementation of os::signal expects that handler argument to be a signal handler, taking 1 argument.  It's not a sigaction handler like POSIX because Windows seemingly doesn't support the sigaction stuff.

This all makes os::signal a quite questionable portability API.  And it turns out all calls to os::signal are in posix-specific or windows-specific files.  So it seems like we don't even need this portability API.  Probably os::signal should be removed entirely, and windows and posix code should each have their own signal handler manipulation functions, and never the twain shall meet.

There are also some problematic uses.  For example, in os_windows.cpp there is a call to os::signal with the handler being UserHandler, a function of 3 arguments (though only the first is used), which will be called with only one argument.  All the other uses need to be examined.
Comments
Changeset: 9b9be88b Author: David Holmes <dholmes@openjdk.org> Date: 2022-10-31 05:55:54 +0000 URL: https://git.openjdk.org/jdk/commit/9b9be88bcaa35c89b6915ff0c251e5a04b10b330
31-10-2022
A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/10780 Date: 2022-10-20 05:59:33 +0000
20-10-2022
We can move some of the signal API's out of os.hpp and then streamline the code in both Posix and Windows, but the cleanup is not as extensive as first envisaged.
20-10-2022
> The posix implementation of os::signal expects that handler argument to be a sigaction handler, taking 3 arguments. [~kbarrett] The handler argument can also be SIG_DFL or SIG_IGN or an arbitrary signal handling function that the Java code is trying to restore - the type of which we have no idea.
17-10-2022
Edit: The original comment here was wrong. This gets even worse when you look at the callers of os::signal and how things come in from the Java side. The sun.misc.Signal API is fundamentally broken: it allows you to register a Java level callback by installing the os::user_handler() as the actual signal handling function, and return the previous handler as a jlong "address". That part is fine, but if the application logic then tries to re-install that previous handler, the VM just gets an address and has no idea whether this should be set via sa_handler or sa_sigaction, nor whether to set SA_SIGINFO! For VM-level signal chaining we actually save the sigact structures associated with a signal - and for the JDK sun.misc.Signal we would have to do the same, and somehow persist those structures - but we would never know when we could free them again.
17-10-2022