RFR: 8252324: Signal related code should be shared among POSIX platforms
Doerr, Martin
martin.doerr at sap.com
Wed Sep 23 09:51:15 UTC 2020
Sorry, Gerard's email address was wrong.
> -----Original Message-----
> From: Doerr, Martin
> Sent: Mittwoch, 23. September 2020 11:40
> To: Gerard Ziemski <gziemski at openjdk.java.net>; hotspot-runtime-
> dev at openjdk.java.net
> Cc: Stuefe, Thomas <thomas.stuefe at sap.com>
> Subject: RE: RFR: 8252324: Signal related code should be shared among POSIX
> platforms
>
> Hi Gerard,
>
> sorry for the long delay. It took time to get our nightly tests working again on
> AIX.
> I have seen an issue, but it may be unrelated to your change. We'll retest it.
>
> Note that there's still unused code left in os_aix.cpp (see below).
> Thanks again for taking care of AIX. I appreciate having more shared POSIX
> code.
>
> Best regards,
> Martin
>
>
> diff --git a/src/hotspot/os/aix/os_aix.cpp b/src/hotspot/os/aix/os_aix.cpp
> index 02568cf..eb84217 100644
> --- a/src/hotspot/os/aix/os_aix.cpp
> +++ b/src/hotspot/os/aix/os_aix.cpp
> @@ -1550,134 +1550,6 @@ void
> os::print_jni_name_suffix_on(outputStream* st, int args_size) {
> // no suffix required
> }
>
> -
> //////////////////////////////////////////////////////////////////////////////
> //
> -// sun.misc.Signal support
> -
> -static void
> -UserHandler(int sig, void *siginfo, void *context) {
> - // Ctrl-C is pressed during error reporting, likely because the error
> - // handler fails to abort. Let VM die immediately.
> - if (sig == SIGINT && VMError::is_error_reported()) {
> - os::die();
> - }
> -
> - os::signal_notify(sig);
> -}
> -
> -extern "C" {
> - typedef void (*sa_handler_t)(int);
> - typedef void (*sa_sigaction_t)(int, siginfo_t *, void *);
> -}
> -
> -//
> -// The following code is moved from os.cpp for making this
> -// code platform specific, which it is by its very nature.
> -//
> -
> -// a counter for each possible signal value
> -static volatile jint pending_signals[NSIG+1] = { 0 };
> -
> -// Wrapper functions for: sem_init(), sem_post(), sem_wait()
> -// On AIX, we use sem_init(), sem_post(), sem_wait()
> -// On Pase, we need to use msem_lock() and msem_unlock(), because
> Posix Semaphores
> -// do not seem to work at all on PASE (unimplemented, will cause SIGILL).
> -// Note that just using msem_.. APIs for both PASE and AIX is not an option
> either, as
> -// on AIX, msem_..() calls are suspected of causing problems.
> -static sem_t sig_sem;
> -static msemaphore* p_sig_msem = 0;
> -
> -static void local_sem_init() {
> - if (os::Aix::on_aix()) {
> - int rc = ::sem_init(&sig_sem, 0, 0);
> - guarantee(rc != -1, "sem_init failed");
> - } else {
> - // Memory semaphores must live in shared mem.
> - guarantee0(p_sig_msem == NULL);
> - p_sig_msem =
> (msemaphore*)os::reserve_memory(sizeof(msemaphore), NULL);
> - guarantee(p_sig_msem, "Cannot allocate memory for memory
> semaphore");
> - guarantee(::msem_init(p_sig_msem, 0) == p_sig_msem, "msem_init
> failed");
> - }
> -}
> -
> -static void local_sem_post() {
> - static bool warn_only_once = false;
> - if (os::Aix::on_aix()) {
> - int rc = ::sem_post(&sig_sem);
> - if (rc == -1 && !warn_only_once) {
> - trcVerbose("sem_post failed (errno = %d, %s)", errno,
> os::errno_name(errno));
> - warn_only_once = true;
> - }
> - } else {
> - guarantee0(p_sig_msem != NULL);
> - int rc = ::msem_unlock(p_sig_msem, 0);
> - if (rc == -1 && !warn_only_once) {
> - trcVerbose("msem_unlock failed (errno = %d, %s)", errno,
> os::errno_name(errno));
> - warn_only_once = true;
> - }
> - }
> -}
> -
> -static void local_sem_wait() {
> - static bool warn_only_once = false;
> - if (os::Aix::on_aix()) {
> - int rc = ::sem_wait(&sig_sem);
> - if (rc == -1 && !warn_only_once) {
> - trcVerbose("sem_wait failed (errno = %d, %s)", errno,
> os::errno_name(errno));
> - warn_only_once = true;
> - }
> - } else {
> - guarantee0(p_sig_msem != NULL); // must init before use
> - int rc = ::msem_lock(p_sig_msem, 0);
> - if (rc == -1 && !warn_only_once) {
> - trcVerbose("msem_lock failed (errno = %d, %s)", errno,
> os::errno_name(errno));
> - warn_only_once = true;
> - }
> - }
> -}
> -
> -static void jdk_misc_signal_init() {
> - // Initialize signal structures
> - ::memset((void*)pending_signals, 0, sizeof(pending_signals));
> -
> - // Initialize signal semaphore
> - local_sem_init();
> -}
> -
> -static int check_pending_signals() {
> - for (;;) {
> - for (int i = 0; i < NSIG + 1; i++) {
> - jint n = pending_signals[i];
> - if (n > 0 && n == Atomic::cmpxchg(&pending_signals[i], n, n - 1)) {
> - return i;
> - }
> - }
> - JavaThread *thread = JavaThread::current();
> - ThreadBlockInVM tbivm(thread);
> -
> - bool threadIsSuspended;
> - do {
> - thread->set_suspend_equivalent();
> - // cleared by handle_special_suspend_equivalent_condition() or
> java_suspend_self()
> -
> - local_sem_wait();
> -
> - // were we externally suspended while we were waiting?
> - threadIsSuspended = thread-
> >handle_special_suspend_equivalent_condition();
> - if (threadIsSuspended) {
> - //
> - // The semaphore has been incremented, but while we were waiting
> - // another thread suspended us. We don't want to continue running
> - // while suspended because that would surprise the thread that
> - // suspended us.
> - //
> -
> - local_sem_post();
> -
> - thread->java_suspend_self();
> - }
> - } while (threadIsSuspended);
> - }
> -}
>
>
> //////////////////////////////////////////////////////////////////////////////
> //
> // Virtual Memory
>
>
>
> > -----Original Message-----
> > From: hotspot-runtime-dev <hotspot-runtime-dev-
> retn at openjdk.java.net>
> > On Behalf Of Gerard Ziemski
> > Sent: Mittwoch, 16. September 2020 18:00
> > To: hotspot-runtime-dev at openjdk.java.net
> > Subject: RFR: 8252324: Signal related code should be shared among POSIX
> > platforms
> >
> > 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();
> >
> > -------------
> >
> > Commit messages:
> > - removed white spaces
> > - Refactored common POSIX signal code into seperate file
> >
> > Changes: https://git.openjdk.java.net/jdk/pull/157/files
> > Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=157&range=00
> > Issue: https://bugs.openjdk.java.net/browse/JDK-8252324
> > Stats: 5247 lines in 21 files changed: 1740 ins; 3400 del; 107 mod
> > Patch: https://git.openjdk.java.net/jdk/pull/157.diff
> > Fetch: git fetch https://git.openjdk.java.net/jdk pull/157/head:pull/157
> >
> > PR: https://git.openjdk.java.net/jdk/pull/157
More information about the hotspot-runtime-dev
mailing list