RFR: 8253742: POSIX signal code cleanup

[David Holmes]

On Mon, 2 Nov 2020 20:13:49 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> hi all,
>> 
>> Please review this followup to [JDK-8252324 Signal related code should be shared among POSIX platforms](https://bugs.openjdk.java.net/browse/JDK-8252324), where several issues were identified for a cleanup. This change addresses them all:
>> 
>> #1 David's feedback - removed non POSIX SIGNIFICANT_SIGNAL_MASK code
>> 
>> #2 David's feedback - used unblock_program_error_signals() on all platforms (this change is superseeded by JDK-8252533, will need to merge)
>> 
>> #3 David's feedback - used single JVM_handle_posix_signal API for all POSIX platforms
>> 
>> #4 Coleen's feedback - cleanup header files in src/hotspot/os/posix/signals_posix.hpp
>> 
>> #5 Coleen's feedback - hid SR_signum assignment in src/hotspot/os/posix/signals_posix.hpp to avoid having to include <signal.h>
>> 
>> #6 Coleen's feedback - factored out print_signal_handlers()
>> 
>> #7 Thomas' feedback - factored out common POSIX os::SuspendedThreadTask::internal_do_task()
>> 
>> #8 Thomas's feedback - factored out common POSIX signal initialization code
>> 
>> #9 YaSuenag's feedback - used JVM_handle_posix_signal for the common API
>> 
>> #10 YaSuenag's feedback - unified logging out of the scope for this fix
>> 
>> #11 YaSuenag's feedback - memset usage in PosixSignals::jdk_misc_signal_init() correct?
>
> Hi Gerard,
> 
> good job!
> 
> But we really should synchronize  :)
> 
> I am currently working on: https://bugs.openjdk.java.net/browse/JDK-8255711
> (see Draft PR: https://github.com/openjdk/jdk/pull/982)
> 
> and https://bugs.openjdk.java.net/browse/JDK-8252533 is also still open (see https://github.com/openjdk/jdk/pull/839) - still waiting Davids final OK.
> 
> So unfortunately there are a number of clashes with your change:
> 
> - "JVM_handle_xxx_signal()": See my mail to jdk-dev from this morning: https://mail.openjdk.java.net/pipermail/jdk-dev/2020-November/004887.html - we either can get completely rid of this function, which is what my current Draft for JDK-8255711 does. Or we need to retain it for backward compatibility, but if so, we need to retain it with the current interface.
> 
> Either way, could you please withhold changes to the hotspot signal handlers for the moment (so, both javaSignalHandler() and the various JVM_handle_xxx_signal() functions)?
> 
> - I removed some functions you changed - all that signal blocking mask stuff. See https://github.com/openjdk/jdk/pull/839. Could you hold any changes to those functions until JDK-8252533 is out of the door? Hope to do this tomorrow, with your and Davids approval.
> 
> The unification of the signal handler printing stuff and the SR initialization make sense and are a good simplification.
>  
> Please fine further remarks remarks inline.
> 
> Cheers, Thomas

I hadn't realized that JVM_handle_XXX_signal defined a per-platform "public" entry point to allow external callers of the signal handling function in conjunction with -XX:+AllowUserSignalHandlers. We need to keep these but they can each call JVM_handle_posix_signal as their implementation.

-------------

PR: https://git.openjdk.java.net/jdk/pull/636