RFR: 8252324: Signal related code should be shared among POSIX platforms
Doerr, Martin
martin.doerr at sap.com
Wed Sep 23 09:39:37 UTC 2020
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