RFR: 8252324: Signal related code should be shared among POSIX platforms [v5]

[Thomas Stuefe]

On Mon, 28 Sep 2020 20:22:56 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> hi all,
>> 
>> Please review this change that refactors common POSIX code into a separate
>> file.
>> 
>> Currently there appears to be quite a bit of duplicated code among POSIX
>> platforms, which makes it difficult to apply single fix to the signal code.
>> With this fix, we will only need to touch single file for common POSIX
>> code fixes from now on.
>> 
>> ----------------------------------------------------------------------------
>> The APIs which moved from os/bsd/os_bsd.cpp to to os/posix/PosixSignals.cpp:
>> 
>> ////////////////////////////////////////////////////////////////////////////////
>> // signal support
>> void os::Bsd::signal_sets_init()
>> sigset_t* os::Bsd::unblocked_signals()
>> sigset_t* os::Bsd::vm_signals()
>> void os::Bsd::hotspot_sigmask(Thread* thread)
>> ////////////////////////////////////////////////////////////////////////////////
>> // sun.misc.Signal support
>> static void UserHandler(int sig, void *siginfo, void *context)
>> void* os::user_handler()
>> void* os::signal(int signal_number, void* handler)
>> void os::signal_raise(int signal_number)
>> int os::sigexitnum_pd()
>> static void jdk_misc_signal_init()
>> void os::signal_notify(int sig)
>> static int check_pending_signals()
>> int os::signal_wait()
>> ////////////////////////////////////////////////////////////////////////////////
>> // suspend/resume support
>> static void resume_clear_context(OSThread *osthread)
>> static void suspend_save_context(OSThread *osthread, siginfo_t* siginfo, ucontext_t* context)
>> static void SR_handler(int sig, siginfo_t* siginfo, ucontext_t* context)
>> static int SR_initialize()
>> static int sr_notify(OSThread* osthread)
>> static bool do_suspend(OSThread* osthread)
>> static void do_resume(OSThread* osthread)
>> ///////////////////////////////////////////////////////////////////////////////////
>> // signal handling (except suspend/resume)
>> static void signalHandler(int sig, siginfo_t* info, void* uc)
>> struct sigaction* os::Bsd::get_chained_signal_action(int sig)
>> static bool call_chained_handler(struct sigaction *actp, int sig,
>>                                  siginfo_t *siginfo, void *context)
>> bool os::Bsd::chained_handler(int sig, siginfo_t* siginfo, void* context)
>> int os::Bsd::get_our_sigflags(int sig)
>> void os::Bsd::set_our_sigflags(int sig, int flags)
>> void os::Bsd::set_signal_handler(int sig, bool set_installed)
>> void os::Bsd::install_signal_handlers()
>> static const char* get_signal_handler_name(address handler,
>>                                            char* buf, int buflen)
>> static void print_signal_handler(outputStream* st, int sig,
>>                                  char* buf, size_t buflen)
>> void os::run_periodic_checks()
>> void os::Bsd::check_signal_handler(int sig)
>> 
>> -----------------------------------------------------------------------------
>> The APIs which moved from os/posix/os_posix.cpp to os/posix/PosixSignals.cpp:
>> 
>> const char* os::Posix::get_signal_name(int sig, char* out, size_t outlen)
>> int os::Posix::get_signal_number(const char* signal_name)
>> int os::get_signal_number(const char* signal_name)
>> bool os::Posix::is_valid_signal(int sig)
>> bool os::Posix::is_sig_ignored(int sig)
>> const char* os::exception_name(int sig, char* buf, size_t size)
>> const char* os::Posix::describe_signal_set_short(const sigset_t* set, char* buffer, size_t buf_size)
>> void os::Posix::print_signal_set_short(outputStream* st, const sigset_t* set)
>> const char* os::Posix::describe_sa_flags(int flags, char* buffer, size_t size)
>> oid os::Posix::print_sa_flags(outputStream* st, int flags)
>> static bool get_signal_code_description(const siginfo_t* si, enum_sigcode_desc_t* out)
>> void os::print_siginfo(outputStream* os, const void* si0)
>> bool os::signal_thread(Thread* thread, int sig, const char* reason)
>> int os::Posix::unblock_thread_signal_mask(const sigset_t *set)
>> address os::Posix::ucontext_get_pc(const ucontext_t* ctx)
>> void os::Posix::ucontext_set_pc(ucontext_t* ctx, address pc)
>> struct sigaction* os::Posix::get_preinstalled_handler(int sig)
>> void os::Posix::save_preinstalled_handler(int sig, struct sigaction& oldAct)
>> 
>> 
>> --------------------------------------------------------
>> --------------------------------------------------------
>> 
>> DETAILS:
>> 
>> --------------------------------------------------------
>> Public APIs which are now internal static PosixSignals::
>> 
>> sigset_t* os::Bsd::vm_signals()
>> struct sigaction* os::Bsd::get_chained_signal_action(int sig)
>> int os::Bsd::get_our_sigflags(int sig)
>> void os::Bsd::set_our_sigflags(int sig, int flags)
>> void os::Bsd::set_signal_handler(int sig, bool set_installed)
>> void os::Bsd::check_signal_handler(int sig)
>> const char* os::Posix::get_signal_name(int sig, char* out, size_t outlen)
>> bool os::Posix::is_valid_signal(int sig)
>> const char* os::Posix::describe_signal_set_short(const sigset_t* set, char* buffer, size_t buf_size)
>> void os::Posix::print_signal_set_short(outputStream* st, const sigset_t* set)
>> const char* os::Posix::describe_sa_flags(int flags, char* buffer, size_t size)
>> oid os::Posix::print_sa_flags(outputStream* st, int flags)
>> static bool get_signal_code_description(const siginfo_t* si, enum_sigcode_desc_t* out)
>> void os::Posix::save_preinstalled_handler(int sig, struct sigaction& oldAct)
>> 
>> ------------------------------------------------
>> Public APIs which moved to public PosixSignals::
>> 
>> void os::Bsd::signal_sets_init()
>> void os::Bsd::hotspot_sigmask(Thread* thread)
>> bool os::Bsd::chained_handler(int sig, siginfo_t* siginfo, void* context)
>> void os::Bsd::install_signal_handlers()
>> bool os::Posix::is_sig_ignored(int sig)
>> int os::Posix::unblock_thread_signal_mask(const sigset_t *set)
>> address os::Posix::ucontext_get_pc(const ucontext_t* ctx)
>> void os::Posix::ucontext_set_pc(ucontext_t* ctx, address pc)
>> 
>> ----------------------------------------------------
>> Internal APIs which are now public in PosixSignals::
>> 
>> static void jdk_misc_signal_init()
>> static int SR_initialize()
>> static bool do_suspend(OSThread* osthread)
>> static void do_resume(OSThread* osthread)
>> static void print_signal_handler(outputStream* st, int sig, char* buf, size_t buflen)
>> 
>> --------------------------
>> New APIs in PosixSignals::
>> 
>> static bool are_signal_handlers_installed();
>
> Gerard Ziemski has updated the pull request with a new target base due to a merge or a rebase. The pull request now
> contains six commits:
>  - Merge branch 'master' into JDK-8252324
>  - Revert "Add AIX specific SA code"
>    
>    This reverts commit cc13700d7d3f15927e22d92d9f5ec9a0739ef9a1.
>  - Add AIX specific SA code
>  - Remove leftover AIX signal code
>  - removed white spaces
>  - Refactored common POSIX signal code into seperate file

Gerard, this is a great cleanup. I am fine with the changes.

This will make maintenance of ports easier. As we see right in this patch, platform ports can diverge over time,
especially if they are closed ports which only exist downstream (as the AIX port did for a long time at SAP).

.. Thomas

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

Marked as reviewed by stuefe (Reviewer).

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