From 9e00bc034fec6c6cfd9e01715545693734b9b6dd Mon Sep 17 00:00:00 2001
From: Babneet Singh <sbabneet@ca.ibm.com>
Date: Wed, 25 Apr 2018 11:45:15 -0400
Subject: [PATCH 1/9] Add new OMR Signal API: omrsig_register_os_handler

omrsig_register_os_handler will register a handler with the OS. Stubs
have been added for omrsig_register_os_handler.

On Unix variants, we cache the original OS handler in oldActions while
registering the first handler with the OS. The original OS handler is
restored during shutdown.

If OMRSIG_SIGACTION is called outside OMR, then the original OS handler
will be overwritten without being cached in oldActions. Subsequently,
the oldActions may cache a user handler instead of the original OS
handler. During shutdown, we may not restore the original OS handler
incorrectly.

The new API, omrsig_register_os_handler, will let a user register a
handler with the OS while properly caching the original OS handler. This
will allow us to properly restore the original OS handler during
shutdown.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
---
 include_core/omrport.h    |  3 +++
 port/common/omrport.c     |  1 +
 port/common/omrsignal.c   | 45 +++++++++++++++++++++++++++++++++++++++
 port/omrportpriv.h        |  2 ++
 port/unix/omrsignal.c     |  6 ++++++
 port/win32/omrsignal.c    |  6 ++++++
 port/win64amd/omrsignal.c |  6 ++++++
 port/ztpf/omrsignal.c     |  6 ++++++
 8 files changed, 75 insertions(+)

diff --git a/include_core/omrport.h b/include_core/omrport.h
index bc9700879b4..d92f59c78ef 100644
--- a/include_core/omrport.h
+++ b/include_core/omrport.h
@@ -1388,6 +1388,8 @@ typedef struct OMRPortLibrary {
 	uint32_t (*sig_map_os_signal_to_portlib_signal)(struct OMRPortLibrary *portLibrary, uint32_t osSignalValue) ;
 	/** see @ref omrsignal.c::omrsig_map_portlib_signal_to_os_signal "omrsig_map_portlib_signal_to_os_signal"*/
 	int32_t (*sig_map_portlib_signal_to_os_signal)(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag) ;
+	/** see @ref omrsignal.c::omrsig_register_os_handler "omrsig_register_os_handler"*/
+	int32_t (*sig_register_os_handler)(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag, void *newOSHandler, void **oldOSHandler) ;
 	/** see @ref omrsignal.c::omrsig_info "omrsig_info"*/
 	uint32_t (*sig_info)(struct OMRPortLibrary *portLibrary, void *info, uint32_t category, int32_t index, const char **name, void **value) ;
 	/** see @ref omrsignal.c::omrsig_info_count "omrsig_info_count"*/
@@ -1889,6 +1891,7 @@ extern J9_CFUNC int32_t omrport_getVersion(struct OMRPortLibrary *portLibrary);
 #define omrsig_set_single_async_signal_handler(param1,param2,param3,param4) privateOmrPortLibrary->sig_set_single_async_signal_handler(privateOmrPortLibrary, (param1), (param2), (param3), (param4))
 #define omrsig_map_os_signal_to_portlib_signal(param1) privateOmrPortLibrary->sig_map_os_signal_to_portlib_signal(privateOmrPortLibrary, (param1))
 #define omrsig_map_portlib_signal_to_os_signal(param1) privateOmrPortLibrary->sig_map_portlib_signal_to_os_signal(privateOmrPortLibrary, (param1))
+#define omrsig_register_os_handler(param1,param2,param3) privateOmrPortLibrary->sig_register_os_handler(privateOmrPortLibrary, (param1), (param2), (param3))
 #define omrsig_info(param1,param2,param3,param4,param5) privateOmrPortLibrary->sig_info(privateOmrPortLibrary, (param1), (param2), (param3), (param4), (param5))
 #define omrsig_info_count(param1,param2) privateOmrPortLibrary->sig_info_count(privateOmrPortLibrary, (param1), (param2))
 #define omrsig_set_options(param1) privateOmrPortLibrary->sig_set_options(privateOmrPortLibrary, (param1))
diff --git a/port/common/omrport.c b/port/common/omrport.c
index f9197966e07..ba312269b55 100644
--- a/port/common/omrport.c
+++ b/port/common/omrport.c
@@ -219,6 +219,7 @@ static OMRPortLibrary MasterPortLibraryTable = {
 	omrsig_set_single_async_signal_handler, /* sig_set_single_async_signal_handler */
 	omrsig_map_os_signal_to_portlib_signal, /* sig_map_os_signal_to_portlib_signal */
 	omrsig_map_portlib_signal_to_os_signal, /* sig_map_portlib_signal_to_os_signal */
+	omrsig_register_os_handler, /* sig_register_os_handler */
 	omrsig_info, /* sig_info */
 	omrsig_info_count, /* sig_info_count */
 	omrsig_set_options, /* sig_set_options */
diff --git a/port/common/omrsignal.c b/port/common/omrsignal.c
index 5e1ea57c233..eec38de5168 100644
--- a/port/common/omrsignal.c
+++ b/port/common/omrsignal.c
@@ -225,6 +225,51 @@ omrsig_map_portlib_signal_to_os_signal(struct OMRPortLibrary *portLibrary, uint3
 	return OMRPORT_SIG_ERROR;
 }
 
+/**
+ * @brief Register a handler with the OS. For an invalid portlibSignalFlag, an error is returned.
+ * portlibSignalFlag is invalid: if it is zero or if multiple signal bits are specified or if the
+ * specified flag is not supported. If OS fails to register newOSHandler for the specified signal,
+ * then an error is returned. In error cases, oldOSHandler won't be updated.
+ *
+ * This function may override a master handler which was previously set by omrsig_protect or
+ * omrsig_set_*_async_signal_handler variant. The records associated with the master handler
+ * for the portlibSignalFlag signal are left unchanged when this function overrides the master
+ * handler. When the master handler is re-registered with the portlibSignalFlag signal, then
+ * the records associated with master handler don't need to be restored. An example of records
+ * is J9*AsyncHandlerRecord(s) in asyncHandlerList.
+ *
+ * Each platform variant of omrsignal.c should have a signalMap. signalMap should have a list of
+ * signals, which are supported on a platform. This function supports all signals listed in
+ * omrsignal.c::signalMap.
+ *
+ * On Unix variants, sigaction is used to register the handler. SA_SIGINFO flag is set.
+ * So, the signature of the signal handling function should be void (*)(int, siginfo_t *, void *).
+ * On zLinux, the signature changes to void (*)(int, siginfo_t *, void *, uintptr_t).
+ *
+ * sigaction isn't available on Windows. On Windows, signal is used to register the handler.
+ * The signature of the signal handling function should be void (*)(int).
+ *
+ * On Unix variants, original OS handler is cached in oldActions while registering the first handler
+ * with the OS. The original OS handler is restored during shutdown. If OMRSIG_SIGACTION is called outside
+ * OMR, then the original OS handler can get overwritten without being cached in oldActions. Subsequently,
+ * the oldActions can cache a user handler instead of the original OS handler. During shutdown, the
+ * original OS handler can be restored incorrectly. omrsig_register_os_handler lets a user register a
+ * handler with the OS while properly caching the original OS handler. The original OS handler is properly
+ * restored during shutdown.
+ *
+ * @param[in] portLibrary The port library
+ * @param[in] portlibSignalFlag a single port library signal flag
+ * @param[in] newOSHandler the new signal handler function to register
+ * @param[out] oldOSHandler points to the old signal handler function
+ *
+ * @return 0 on success or non-zero on failure
+ */
+int32_t
+omrsig_register_os_handler(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag, void *newOSHandler, void **oldOSHandler)
+{
+	return OMRPORT_SIG_ERROR;
+}
+
 /**
  * Determine if the port library is capable of protecting a function from the indicated signals in the indicated way.
  *
diff --git a/port/omrportpriv.h b/port/omrportpriv.h
index b28d125c334..9cec3c27b83 100644
--- a/port/omrportpriv.h
+++ b/port/omrportpriv.h
@@ -555,6 +555,8 @@ extern J9_CFUNC uint32_t
 omrsig_map_os_signal_to_portlib_signal(struct OMRPortLibrary *portLibrary, uint32_t osSignalValue);
 extern J9_CFUNC int32_t
 omrsig_map_portlib_signal_to_os_signal(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag);
+extern J9_CFUNC int32_t
+omrsig_register_os_handler(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag, void *newOSHandler, void **oldOSHandler);
 extern J9_CFUNC uint32_t
 omrsig_info(struct OMRPortLibrary *portLibrary, void *info, uint32_t category, int32_t index, const char **name, void **value);
 extern J9_CFUNC int32_t
diff --git a/port/unix/omrsignal.c b/port/unix/omrsignal.c
index e9b0d244ec5..603f840c558 100644
--- a/port/unix/omrsignal.c
+++ b/port/unix/omrsignal.c
@@ -581,6 +581,12 @@ omrsig_map_portlib_signal_to_os_signal(struct OMRPortLibrary *portLibrary, uint3
 	return (int32_t)mapPortLibSignalToUnix(portlibSignalFlag);
 }
 
+int32_t
+omrsig_register_os_handler(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag, void *newOSHandler, void **oldOSHandler)
+{
+	return OMRPORT_SIG_ERROR;
+}
+
 /*
  * The full shutdown routine "sig_full_shutdown" overwrites this once we've completed startup
  */
diff --git a/port/win32/omrsignal.c b/port/win32/omrsignal.c
index 57a217b943e..0a0d145bf79 100644
--- a/port/win32/omrsignal.c
+++ b/port/win32/omrsignal.c
@@ -197,6 +197,12 @@ omrsig_map_portlib_signal_to_os_signal(struct OMRPortLibrary *portLibrary, uint3
 	return OMRPORT_SIG_ERROR;
 }
 
+int32_t
+omrsig_register_os_handler(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag, void *newOSHandler, void **oldOSHandler)
+{
+	return OMRPORT_SIG_ERROR;
+}
+
 int32_t
 omrsig_can_protect(struct OMRPortLibrary *portLibrary,  uint32_t flags)
 {
diff --git a/port/win64amd/omrsignal.c b/port/win64amd/omrsignal.c
index 1814b449f8c..51ee5f220f3 100644
--- a/port/win64amd/omrsignal.c
+++ b/port/win64amd/omrsignal.c
@@ -317,6 +317,12 @@ omrsig_map_portlib_signal_to_os_signal(struct OMRPortLibrary *portLibrary, uint3
 	return OMRPORT_SIG_ERROR;
 }
 
+int32_t
+omrsig_register_os_handler(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag, void *newOSHandler, void **oldOSHandler)
+{
+	return OMRPORT_SIG_ERROR;
+}
+
 int32_t
 omrsig_can_protect(struct OMRPortLibrary *portLibrary,  uint32_t flags)
 {
diff --git a/port/ztpf/omrsignal.c b/port/ztpf/omrsignal.c
index 5f3f1b83f26..c1493c4d776 100644
--- a/port/ztpf/omrsignal.c
+++ b/port/ztpf/omrsignal.c
@@ -569,6 +569,12 @@ omrsig_map_portlib_signal_to_os_signal(struct OMRPortLibrary *portLibrary, uint3
 	return OMRPORT_SIG_ERROR;
 }
 
+int32_t
+omrsig_register_os_handler(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag, void *newOSHandler, void **oldOSHandler)
+{
+	return OMRPORT_SIG_ERROR;
+}
+
 /*
  * The full shutdown routine "sig_full_shutdown" overrides this once we've completed startup 
  */

From 0657638823fa198323f7c0dff24b14e9c926650f Mon Sep 17 00:00:00 2001
From: Babneet Singh <sbabneet@ca.ibm.com>
Date: Wed, 25 Apr 2018 11:56:16 -0400
Subject: [PATCH 2/9] Re-format comments' layout

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
---
 port/common/omrsignal.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/port/common/omrsignal.c b/port/common/omrsignal.c
index eec38de5168..6dfefe66db4 100644
--- a/port/common/omrsignal.c
+++ b/port/common/omrsignal.c
@@ -172,7 +172,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 }
 
 
-/*
+/**
  * @brief Similar to omrsig_set_async_signal_handler. Refer to omrsig_set_async_signal_handler's description above.
  * A new element is added to the asyncHandlerList for omrsig_handler_fn, and masterASynchSignalHandler is registered
  * with the OS for the signal corresponding to the specified portlibSignalFlag. masterASynchSignalHandler invokes
@@ -197,21 +197,21 @@ omrsig_set_single_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsi
 	return OMRPORT_SIG_ERROR;
 }
 
-/*
+/**
  * @brief Given an OS signal value, return the corresponding port library signal flag.
  *
  * @param[in] portLibrary The port library
  * @param[in] osSignalValue OS signal value
  *
  * @return port library signal flag on success and 0 on failure
-*/
+ */
 uint32_t
 omrsig_map_os_signal_to_portlib_signal(struct OMRPortLibrary *portLibrary, uint32_t osSignalValue)
 {
 	return 0;
 }
 
-/*
+/**
  * @brief Given a port library signal flag, return the corresponding OS signal value.
  *
  * @param[in] portLibrary The port library

From 8cf4c89b1adedb9ca789a67fdd0999b301082d11 Mon Sep 17 00:00:00 2001
From: Babneet Singh <sbabneet@ca.ibm.com>
Date: Wed, 25 Apr 2018 12:20:17 -0400
Subject: [PATCH 3/9] Rename signalsWithMasterHandlers and masterHandlerMonitor

signalsWithMasterHandlers is used to determine if we have registered a
handler with a signal. With the introduction of
omrsig_register_os_handler, a user can register handlers other than the
master handlers. signalsWithMasterHandlers has been renamed to
signalsWithHandlers to better represent its purpose.

masterHandlerMonitor is acquired whenever a handler is registered with
the OS. Since a user can register any arbitrary handler with the OS
using omrsig_register_os_handler, masterHandlerMonitor is renamed to
registerHandlerMonitor to better represent its purpose.

After restoring the original OS handler in sig_full_shutdown,
oldActions[index].restore should be reset to 0.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
---
 port/unix/omrsignal.c | 53 +++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/port/unix/omrsignal.c b/port/unix/omrsignal.c
index 603f840c558..a9f7bc270d1 100644
--- a/port/unix/omrsignal.c
+++ b/port/unix/omrsignal.c
@@ -84,9 +84,10 @@ static struct {
 	uint32_t restore;
 }	oldActions[ARRAY_SIZE_SIGNALS];
 
-/* Records the (port library defined) signals for which we have registered a master handler.
- * Access to this must be protected by the masterHandlerMonitor */
-static uint32_t signalsWithMasterHandlers;
+/* Records the (port library defined) signals for which a handler is registered.
+ * Access to this variable must be protected by the registerHandlerMonitor.
+ */
+static uint32_t signalsWithHandlers;
 
 #if defined(OMR_PORT_ASYNC_HANDLER)
 static uint32_t shutDownASynchReporter;
@@ -141,7 +142,7 @@ static pthread_mutex_t wakeUpASyncReporterMutex;
 #endif /* !defined(J9ZOS390) */
 
 static omrthread_monitor_t asyncMonitor;
-static omrthread_monitor_t masterHandlerMonitor;
+static omrthread_monitor_t registerHandlerMonitor;
 static omrthread_monitor_t asyncReporterShutdownMonitor;
 static uint32_t asyncThreadCount;
 static uint32_t attachedPortLibraries;
@@ -326,10 +327,10 @@ omrsig_protect(struct OMRPortLibrary *portLibrary, omrsig_protected_fn fn, void
 
 	if (0 != flagsSignalsOnly) {
 
-		/* Acquire the masterHandlerMonitor and install the handler via registerMasterHandlers. */
-		omrthread_monitor_enter(masterHandlerMonitor);
+		/* Acquire the registerHandlerMonitor and install the handler via registerMasterHandlers. */
+		omrthread_monitor_enter(registerHandlerMonitor);
 		rc = registerMasterHandlers(portLibrary, flags, OMRPORT_SIG_FLAG_SIGALLSYNC, NULL);
-		omrthread_monitor_exit(masterHandlerMonitor);
+		omrthread_monitor_exit(registerHandlerMonitor);
 
 		if (0 != rc) {
 			return OMRPORT_SIG_ERROR;
@@ -392,7 +393,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 
 	Trc_PRT_signal_omrsig_set_async_signal_handler_entered(handler, handler_arg, flags);
 
-	omrthread_monitor_enter(masterHandlerMonitor);
+	omrthread_monitor_enter(registerHandlerMonitor);
 
 	if (OMR_ARE_ANY_BITS_SET(signalOptionsGlobal, OMRPORT_SIG_OPTIONS_REDUCED_SIGNALS_ASYNCHRONOUS)) {
 		/* -Xrs was set, we can't protect against any signals, do not install any handlers except SIGXFSZ*/
@@ -405,7 +406,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 	} else {
 		rc = registerMasterHandlers(portLibrary, flags, OMRPORT_SIG_FLAG_SIGALLASYNC, NULL);
 	}
-	omrthread_monitor_exit(masterHandlerMonitor);
+	omrthread_monitor_exit(registerHandlerMonitor);
 
 	if (0 != rc) {
 		Trc_PRT_signal_omrsig_set_async_signal_handler_exiting_did_nothing_possible_error(handler, handler_arg, flags);
@@ -489,7 +490,7 @@ omrsig_set_single_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsi
 		}
 	}
 
-	omrthread_monitor_enter(masterHandlerMonitor);
+	omrthread_monitor_enter(registerHandlerMonitor);
 
 	if (OMR_ARE_ANY_BITS_SET(signalOptionsGlobal, OMRPORT_SIG_OPTIONS_REDUCED_SIGNALS_ASYNCHRONOUS)) {
 		/* -Xrs was set, we can't protect against any signals, do not install any handlers except SIGXFSZ*/
@@ -502,7 +503,7 @@ omrsig_set_single_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsi
 	} else {
 		rc = registerMasterHandlers(portLibrary, portlibSignalFlag, OMRPORT_SIG_FLAG_SIGALLASYNC, oldOSHandler);
 	}
-	omrthread_monitor_exit(masterHandlerMonitor);
+	omrthread_monitor_exit(registerHandlerMonitor);
 
 	if (0 != rc) {
 		Trc_PRT_signal_omrsig_set_single_async_signal_handler_exiting_did_nothing_possible_error(rc, handler, handler_arg, portlibSignalFlag);
@@ -1089,7 +1090,7 @@ masterASynchSignalHandler(int signal, siginfo_t *sigInfo, void *contextInfo)
  * Register the signal handler with the OS, generally used to register the master signal handlers
  * Not to be confused with omrsig_protect, which registers the user's handler with the port library.
  *
- * Calls to this function must be synchronized using "masterHandlerMonitor".
+ * Calls to this function must be synchronized using "registerHandlerMonitor".
  *
  * The use of this function forces the flags SA_RESTART | SA_SIGINFO | SA_NODEFER to be set for the new signal action
  *
@@ -1199,11 +1200,12 @@ registerSignalHandlerWithOS(OMRPortLibrary *portLibrary, uint32_t portLibrarySig
 		}
 	}
 
-	/* CMVC 96193 signalsWithMasterHandlers is checked without acquiring the masterHandlerMonitor */
+	/* CMVC 96193 signalsWithHandlers is checked without acquiring the registerHandlerMonitor */
 	issueWriteBarrier();
 
-	/* we've successfully registered the master handler for this, record it! */
-	signalsWithMasterHandlers |= portLibrarySignalNo;
+	/* Set the portLibrarySignalNo bit in signalsWithHandlers to record successful registration
+	 * of the handler. */
+	signalsWithHandlers |= portLibrarySignalNo;
 
 	return 0;
 }
@@ -1307,7 +1309,7 @@ addAsyncSignalsToSet(sigset_t *ss)
 /**
  * Registers the master handler for the signals in flags that don't have one.
  *
- * Calls to this function must be synchronized using masterHandlerMonitor.
+ * Calls to this function must be synchronized using registerHandlerMonitor.
  *
  * @param[in] flags the flags that we want signals for
  * @param[in] allowedSubsetOfFlags must be one of OMRPORT_SIG_FLAG_SIGALLSYNC, or
@@ -1386,7 +1388,7 @@ initializeSignalTools(OMRPortLibrary *portLibrary)
 	}
 #endif
 
-	if (omrthread_monitor_init_with_name(&masterHandlerMonitor, 0, "portLibrary_omrsig_masterHandler_monitor")) {
+	if (omrthread_monitor_init_with_name(&registerHandlerMonitor, 0, "portLibrary_omrsig_registerHandler_monitor")) {
 		return -1;
 	}
 
@@ -1485,7 +1487,7 @@ destroySignalTools(OMRPortLibrary *portLibrary)
 {
 	omrthread_tls_free(tlsKey);
 	omrthread_tls_free(tlsKeyCurrentSignal);
-	omrthread_monitor_destroy(masterHandlerMonitor);
+	omrthread_monitor_destroy(registerHandlerMonitor);
 	omrthread_monitor_destroy(asyncReporterShutdownMonitor);
 	omrthread_monitor_destroy(asyncMonitor);
 #if !defined(J9ZOS390)
@@ -1508,11 +1510,11 @@ omrsig_set_options(struct OMRPortLibrary *portLibrary, uint32_t options)
 
 		uint32_t anyHandlersInstalled = 0;
 
-		omrthread_monitor_enter(masterHandlerMonitor);
-		if (0 != signalsWithMasterHandlers) {
+		omrthread_monitor_enter(registerHandlerMonitor);
+		if (0 != signalsWithHandlers) {
 			anyHandlersInstalled = 1;
 		}
-		omrthread_monitor_exit(masterHandlerMonitor);
+		omrthread_monitor_exit(registerHandlerMonitor);
 
 		if (anyHandlersInstalled) {
 			Trc_PRT_signal_omrsig_set_options_too_late_handlers_installed(options);
@@ -1528,9 +1530,9 @@ omrsig_set_options(struct OMRPortLibrary *portLibrary, uint32_t options)
 
 		int32_t syncHandlersInstalled = 0;
 
-		omrthread_monitor_enter(masterHandlerMonitor);
+		omrthread_monitor_enter(registerHandlerMonitor);
 
-		if (0 == (signalsWithMasterHandlers & OMRPORT_SIG_FLAG_SIGALLSYNC)) {
+		if (OMR_ARE_NO_BITS_SET(signalsWithHandlers, OMRPORT_SIG_FLAG_SIGALLSYNC)) {
 			/* we haven't installed any synchronous handlers, so it's OK to switch to LE condition handling */
 			portLibrary->sig_protect = omrsig_protect_ceehdlr;
 			portLibrary->sig_info = omrsig_info_ceehdlr;
@@ -1540,7 +1542,7 @@ omrsig_set_options(struct OMRPortLibrary *portLibrary, uint32_t options)
 			syncHandlersInstalled = 1;
 		}
 
-		omrthread_monitor_exit(masterHandlerMonitor);
+		omrthread_monitor_exit(registerHandlerMonitor);
 
 		if (syncHandlersInstalled == 1) {
 			Trc_PRT_signal_omrsig_set_options_too_late_handlers_installed(options);
@@ -1588,7 +1590,8 @@ sig_full_shutdown(struct OMRPortLibrary *portLibrary)
 				OMRSIG_SIGACTION(index, &oldActions[index].action, NULL);
 				/* record that we no longer have a handler installed with the OS for this signal */
 				Trc_PRT_signal_sig_full_shutdown_deregistered_handler_with_OS(portLibrary, index);
-				signalsWithMasterHandlers &= ~mapUnixSignalToPortLib(index, 0);
+				signalsWithHandlers &= ~mapUnixSignalToPortLib(index, 0);
+				oldActions[index].restore = 0;
 			}
 		}
 

From 68b356f7f0106d84049f10bc121255bb61aae053 Mon Sep 17 00:00:00 2001
From: Babneet Singh <sbabneet@ca.ibm.com>
Date: Wed, 25 Apr 2018 12:42:20 -0400
Subject: [PATCH 4/9] Add implementation of omrsig_register_os_handler [Unix]

Implementation of omrsig_register_os_handler has been added in
unix/omrsignal.c.

Supporting entry and exit trace points have been added.

registerSignalHandlerWithOS returns OMRPORT_SIG_ERROR for OS signals
that are included in signalMap.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
---
 port/common/omrport.tdf |  6 +++++-
 port/unix/omrsignal.c   | 25 ++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/port/common/omrport.tdf b/port/common/omrport.tdf
index aecc6507ca0..9206ee8a045 100644
--- a/port/common/omrport.tdf
+++ b/port/common/omrport.tdf
@@ -1064,4 +1064,8 @@ TraceEvent=Trc_PRT_signal_omrsig_set_single_async_signal_handler_user_handler_ad
 TraceEvent=Trc_PRT_signal_omrsig_set_single_async_signal_handler_user_handler_added_2 Group=signal Overhead=1 Level=1 NoEnv Template="omrsig_set_single_async_signal_handler: user handler added_2, handler=%p, handler_arg=%p, portlibSignalFlag=0x%X"
 TraceExit=Trc_PRT_signal_omrsig_set_single_async_signal_handler_exiting Group=signal Overhead=1 Level=1 NoEnv Template="omrsig_set_single_async_signal_handler: Exiting, rc=%d, handler=%p, handler_arg=%p, portlibSignalFlag=0x%X, oldOSHandler=%p"
 
-TraceException=Trc_PRT_signal_mapOSSignalToPortLib_ERROR_unknown_signal Group=signal Overhead=1 Level=1 NoEnv Template="omrsignal: ERROR unknown OS signal=0x%X"
+TraceException=Trc_PRT_signal_mapOSSignalToPortLib_ERROR_unknown_signal Group=signal Overhead=1 Level=1 NoEnv Template="omrsignal: ERROR unknown OS signal=0x%X"
+
+TraceEntry=Trc_PRT_signal_omrsig_register_os_handler_entered Group=signal Overhead=1 Level=1 NoEnv Template="omrsig_register_os_handler: Entered, portLibrarySignalFlag=0x%X, newOSHandler=%p"
+TraceExit=Trc_PRT_signal_omrsig_register_os_handler_exiting Group=signal Overhead=1 Level=1 NoEnv Template="omrsig_register_os_handler: Exiting, rc=%d, portLibrarySignalFlag=0x%X, newOSHandler=%p, oldOSHandler=%p"
+TraceException=Trc_PRT_signal_omrsig_register_os_handler_invalid_portlibSignalFlag Group=signal Overhead=1 Level=1 NoEnv Template="omrsig_register_os_handler: ERROR, portlibSignalFlag=0x%X is either zero or has multiple signal bits set."
diff --git a/port/unix/omrsignal.c b/port/unix/omrsignal.c
index a9f7bc270d1..76a00e88814 100644
--- a/port/unix/omrsignal.c
+++ b/port/unix/omrsignal.c
@@ -585,7 +585,23 @@ omrsig_map_portlib_signal_to_os_signal(struct OMRPortLibrary *portLibrary, uint3
 int32_t
 omrsig_register_os_handler(struct OMRPortLibrary *portLibrary, uint32_t portlibSignalFlag, void *newOSHandler, void **oldOSHandler)
 {
-	return OMRPORT_SIG_ERROR;
+	int32_t rc = 0;
+
+	Trc_PRT_signal_omrsig_register_os_handler_entered(portlibSignalFlag, newOSHandler);
+
+	if ((0 == portlibSignalFlag) || !OMR_IS_ONLY_ONE_BIT_SET(portlibSignalFlag)) {
+		/* If portlibSignalFlag is 0 or if portlibSignalFlag has multiple signal bits set, then fail. */
+		Trc_PRT_signal_omrsig_register_os_handler_invalid_portlibSignalFlag(portlibSignalFlag);
+		rc = OMRPORT_SIG_ERROR;
+	} else {
+		omrthread_monitor_enter(registerHandlerMonitor);
+		rc = registerSignalHandlerWithOS(portLibrary, portlibSignalFlag, (unix_sigaction)newOSHandler, oldOSHandler);
+		omrthread_monitor_exit(registerHandlerMonitor);
+	}
+
+	Trc_PRT_signal_omrsig_register_os_handler_exiting(rc, portlibSignalFlag, newOSHandler, *oldOSHandler);
+
+	return rc;
 }
 
 /*
@@ -1109,6 +1125,13 @@ registerSignalHandlerWithOS(OMRPortLibrary *portLibrary, uint32_t portLibrarySig
 	int unixSignalNo = mapPortLibSignalToUnix(portLibrarySignalNo);
 	struct sigaction newAction;
 
+	/* Don't register a handler for unrecognized OS signals.
+	 * Unrecognized OS signals are the ones which aren't included in signalMap.
+	 */
+	if (OMRPORT_SIG_ERROR == unixSignalNo) {
+		return OMRPORT_SIG_ERROR;
+	}
+
 	memset(&newAction, 0, sizeof(struct sigaction));
 
 	/* do not block any signals */

From 6e574d316c48fa25a39bb05524c20fcd89f9878c Mon Sep 17 00:00:00 2001
From: Babneet Singh <sbabneet@ca.ibm.com>
Date: Thu, 26 Apr 2018 11:28:17 -0400
Subject: [PATCH 5/9] Fix omrsig_set_async_signal_handler signature

Currently, omrsig_set_async_signal_handler returns an uint32_t.

But, unix and ztpf implementations of omrsig_set_async_signal_handler
set return value to OMRPORT_SIG_ERROR (-1).

In order to allow negative return values,
omrsig_set_async_signal_handler's signature is changed to return an
int32_t.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
---
 fvtest/porttest/omrsignalTest.cpp | 2 +-
 include_core/omrport.h            | 2 +-
 port/common/omrsignal.c           | 6 +++---
 port/omrportpriv.h                | 2 +-
 port/unix/omrsignal.c             | 4 ++--
 port/win32/omrsignal.c            | 5 ++---
 port/win64amd/omrsignal.c         | 4 ++--
 port/ztpf/omrsignal.c             | 4 ++--
 8 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/fvtest/porttest/omrsignalTest.cpp b/fvtest/porttest/omrsignalTest.cpp
index b35181bb538..4b7a5af98a3 100644
--- a/fvtest/porttest/omrsignalTest.cpp
+++ b/fvtest/porttest/omrsignalTest.cpp
@@ -1100,7 +1100,7 @@ TEST(PortSigTest, sig_test_async_unix_handler)
 	OMRPORT_ACCESS_FROM_OMRPORT(portTestEnv->getPortLibrary());
 	const char *testName = "omrsig_test_async_unix_handler";
 	AsyncHandlerInfo handlerInfo;
-	uint32_t setAsyncRC;
+	int32_t setAsyncRC;
 	unsigned int index;
 	omrthread_monitor_t asyncMonitor;
 	intptr_t monitorRC;
diff --git a/include_core/omrport.h b/include_core/omrport.h
index d92f59c78ef..e7792a7f608 100644
--- a/include_core/omrport.h
+++ b/include_core/omrport.h
@@ -1381,7 +1381,7 @@ typedef struct OMRPortLibrary {
 	/** see @ref omrsignal.c::omrsig_can_protect "omrsig_can_protect"*/
 	int32_t (*sig_can_protect)(struct OMRPortLibrary *portLibrary,  uint32_t flags) ;
 	/** see @ref omrsignal.c::omrsig_set_async_signal_handler "omrsig_set_async_signal_handler"*/
-	uint32_t (*sig_set_async_signal_handler)(struct OMRPortLibrary *portLibrary, omrsig_handler_fn handler, void *handler_arg, uint32_t flags) ;
+	int32_t (*sig_set_async_signal_handler)(struct OMRPortLibrary *portLibrary, omrsig_handler_fn handler, void *handler_arg, uint32_t flags) ;
 	/** see @ref omrsignal.c::omrsig_set_single_async_signal_handler "omrsig_set_single_async_signal_handler"*/
 	int32_t (*sig_set_single_async_signal_handler)(struct OMRPortLibrary *portLibrary, omrsig_handler_fn handler, void *handler_arg, uint32_t portlibSignalFlag, void **oldOSHandler) ;
 	/** see @ref omrsignal.c::omrsig_map_os_signal_to_portlib_signal "omrsig_map_os_signal_to_portlib_signal"*/
diff --git a/port/common/omrsignal.c b/port/common/omrsignal.c
index 6dfefe66db4..daefeda235d 100644
--- a/port/common/omrsignal.c
+++ b/port/common/omrsignal.c
@@ -163,12 +163,12 @@ omrsig_protect(struct OMRPortLibrary *portLibrary,  omrsig_protected_fn fn, void
  * @note A handler should not do anything to cause the reporting thread to terminate (e.g. call omrthread_exit()) as that may prevent
  *  future signals from being reported.
  *
- * @return 0 on success
+ * @return 0 on success or non-zero on failure
  */
-uint32_t
+int32_t
 omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handler_fn handler, void *handler_arg, uint32_t flags)
 {
-	return 1;
+	return OMRPORT_SIG_ERROR;
 }
 
 
diff --git a/port/omrportpriv.h b/port/omrportpriv.h
index 9cec3c27b83..c6d8c5b0d81 100644
--- a/port/omrportpriv.h
+++ b/port/omrportpriv.h
@@ -547,7 +547,7 @@ extern J9_CFUNC int32_t
 omrsig_set_options(struct OMRPortLibrary *portLibrary, uint32_t options);
 extern J9_CFUNC int32_t
 omrsig_protect(struct OMRPortLibrary *portLibrary,  omrsig_protected_fn fn, void *fn_arg, omrsig_handler_fn handler, void *handler_arg, uint32_t flags, uintptr_t *result);
-extern J9_CFUNC uint32_t
+extern J9_CFUNC int32_t
 omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handler_fn handler, void *handler_arg, uint32_t flags);
 extern J9_CFUNC int32_t
 omrsig_set_single_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handler_fn handler, void *handler_arg, uint32_t portlibSignalFlag, void **oldOSHandler);
diff --git a/port/unix/omrsignal.c b/port/unix/omrsignal.c
index 76a00e88814..beb0e4a2a86 100644
--- a/port/unix/omrsignal.c
+++ b/port/unix/omrsignal.c
@@ -384,10 +384,10 @@ omrsig_protect(struct OMRPortLibrary *portLibrary, omrsig_protected_fn fn, void
 	return 0;
 }
 
-uint32_t
+int32_t
 omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handler_fn handler, void *handler_arg, uint32_t flags)
 {
-	uint32_t rc = 0;
+	int32_t rc = 0;
 	J9UnixAsyncHandlerRecord *cursor = NULL;
 	J9UnixAsyncHandlerRecord **previousLink = NULL;
 
diff --git a/port/win32/omrsignal.c b/port/win32/omrsignal.c
index 0a0d145bf79..a152cd2373a 100644
--- a/port/win32/omrsignal.c
+++ b/port/win32/omrsignal.c
@@ -106,11 +106,10 @@ omrsig_protect(struct OMRPortLibrary *portLibrary,  omrsig_protected_fn fn, void
 	return 0;
 }
 
-uint32_t
+int32_t
 omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handler_fn handler, void *handler_arg, uint32_t flags)
 {
-
-	uint32_t rc = 0;
+	int32_t rc = 0;
 	J9Win32AsyncHandlerRecord *cursor;
 	J9Win32AsyncHandlerRecord **previousLink;
 
diff --git a/port/win64amd/omrsignal.c b/port/win64amd/omrsignal.c
index 51ee5f220f3..4f60a6ecedd 100644
--- a/port/win64amd/omrsignal.c
+++ b/port/win64amd/omrsignal.c
@@ -228,10 +228,10 @@ omrsig_protect(struct OMRPortLibrary *portLibrary, omrsig_protected_fn fn, void
 	return 0;
 }
 
-uint32_t
+int32_t
 omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handler_fn handler, void *handler_arg, uint32_t flags)
 {
-	uint32_t rc = 0;
+	int32_t rc = 0;
 	J9WinAMD64AsyncHandlerRecord *cursor;
 	J9WinAMD64AsyncHandlerRecord **previousLink;
 
diff --git a/port/ztpf/omrsignal.c b/port/ztpf/omrsignal.c
index c1493c4d776..7d7ce77953f 100644
--- a/port/ztpf/omrsignal.c
+++ b/port/ztpf/omrsignal.c
@@ -457,11 +457,11 @@ omrsig_protect(struct OMRPortLibrary *portLibrary,  omrsig_protected_fn fn, void
 }
 
 
-uint32_t
+int32_t
 omrsig_set_async_signal_handler(struct OMRPortLibrary* portLibrary, omrsig_handler_fn handler,
 							   void* handler_arg, uint32_t flags)
 {
-	uint32_t rc = 0;
+	int32_t rc = 0;
 	J9UnixAsyncHandlerRecord *cursor;
 	J9UnixAsyncHandlerRecord **previousLink;
 

From 2a53c741ed4f09e2f176a0825378886341e81a16 Mon Sep 17 00:00:00 2001
From: Babneet Singh <sbabneet@ca.ibm.com>
Date: Thu, 3 May 2018 11:17:25 -0400
Subject: [PATCH 6/9] Use error macros instead of undocumented constants for
 errors

In omrsig_set_async_signal_handler and
omrsig_set_single_async_signal_handler, constants such as 1 and -1 are
directly used as error codes. Using constants directly leads to
inconsistency in implementation across platforms.

Now onwards, omrsig_set_async_signal_handler and
omrsig_set_single_async_signal_handler will consistently return
OMRPORT_SIG_ERROR (-1) for error.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
---
 port/unix/omrsignal.c     | 8 ++++----
 port/win32/omrsignal.c    | 4 ++--
 port/win64amd/omrsignal.c | 4 ++--
 port/ztpf/omrsignal.c     | 6 +++---
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/port/unix/omrsignal.c b/port/unix/omrsignal.c
index beb0e4a2a86..1091d9ce38e 100644
--- a/port/unix/omrsignal.c
+++ b/port/unix/omrsignal.c
@@ -401,7 +401,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 			rc = registerMasterHandlers(portLibrary, OMRPORT_SIG_FLAG_SIGXFSZ, OMRPORT_SIG_FLAG_SIGALLASYNC, NULL);
 		} else {
 			Trc_PRT_signal_omrsig_set_async_signal_handler_will_not_set_handler_due_to_Xrs(handler, handler_arg, flags);
-			rc = -1;
+			rc = OMRPORT_SIG_ERROR;
 		}
 	} else {
 		rc = registerMasterHandlers(portLibrary, flags, OMRPORT_SIG_FLAG_SIGALLASYNC, NULL);
@@ -410,7 +410,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 
 	if (0 != rc) {
 		Trc_PRT_signal_omrsig_set_async_signal_handler_exiting_did_nothing_possible_error(handler, handler_arg, flags);
-		return OMRPORT_SIG_ERROR;
+		return rc;
 	}
 
 	omrthread_monitor_enter(asyncMonitor);
@@ -451,7 +451,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 			J9UnixAsyncHandlerRecord *record = portLibrary->mem_allocate_memory(portLibrary, sizeof(*record), OMR_GET_CALLSITE(), OMRMEM_CATEGORY_PORT_LIBRARY);
 
 			if (NULL == record) {
-				rc = 1;
+				rc = OMRPORT_SIG_ERROR;
 			} else {
 				record->portLib = portLibrary;
 				record->handler = handler;
@@ -550,7 +550,7 @@ omrsig_set_single_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsi
 	if (!foundHandler && (0 != portlibSignalFlag)) {
 		J9UnixAsyncHandlerRecord *record = portLibrary->mem_allocate_memory(portLibrary, sizeof(*record), OMR_GET_CALLSITE(), OMRMEM_CATEGORY_PORT_LIBRARY);
 		if (NULL == record) {
-			rc = 1;
+			rc = OMRPORT_SIG_ERROR;
 		} else {
 			record->portLib = portLibrary;
 			record->handler = handler;
diff --git a/port/win32/omrsignal.c b/port/win32/omrsignal.c
index a152cd2373a..c63b3242da7 100644
--- a/port/win32/omrsignal.c
+++ b/port/win32/omrsignal.c
@@ -115,7 +115,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 
 	if (OMRPORT_SIG_OPTIONS_REDUCED_SIGNALS_ASYNCHRONOUS & signalOptions) {
 		/* -Xrs was set, do not install any handlers */
-		return 1;
+		return OMRPORT_SIG_ERROR;
 	}
 
 	omrthread_monitor_enter(asyncMonitor);
@@ -153,7 +153,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 			J9Win32AsyncHandlerRecord *record = portLibrary->mem_allocate_memory(portLibrary, sizeof(*record), OMR_GET_CALLSITE(), OMRMEM_CATEGORY_PORT_LIBRARY);
 
 			if (record == NULL) {
-				rc = 1;
+				rc = OMRPORT_SIG_ERROR;
 			} else {
 				record->portLib = portLibrary;
 				record->handler = handler;
diff --git a/port/win64amd/omrsignal.c b/port/win64amd/omrsignal.c
index 4f60a6ecedd..e9078f7c276 100644
--- a/port/win64amd/omrsignal.c
+++ b/port/win64amd/omrsignal.c
@@ -237,7 +237,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 
 	if (OMRPORT_SIG_OPTIONS_REDUCED_SIGNALS_ASYNCHRONOUS & signalOptions) {
 		/* -Xrs was set, do not install any handlers */
-		return 1;
+		return OMRPORT_SIG_ERROR;
 	}
 
 	omrthread_monitor_enter(asyncMonitor);
@@ -275,7 +275,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
 			J9WinAMD64AsyncHandlerRecord *record = portLibrary->mem_allocate_memory(portLibrary, sizeof(*record), OMR_GET_CALLSITE(), OMRMEM_CATEGORY_PORT_LIBRARY);
 
 			if (record == NULL) {
-				rc = 1;
+				rc = OMRPORT_SIG_ERROR;
 			} else {
 				record->portLib = portLibrary;
 				record->handler = handler;
diff --git a/port/ztpf/omrsignal.c b/port/ztpf/omrsignal.c
index 7d7ce77953f..024cd02b216 100644
--- a/port/ztpf/omrsignal.c
+++ b/port/ztpf/omrsignal.c
@@ -479,7 +479,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary* portLibrary, omrsig_handl
 			rc = registerMasterHandlers(portLibrary, OMRPORT_SIG_FLAG_SIGXFSZ, OMRPORT_SIG_FLAG_SIGALLASYNC);
 		} else {
 			Trc_PRT_signal_omrsig_set_async_signal_handler_will_not_set_handler_due_to_Xrs(handler, handler_arg, flags);
-			rc = -1;
+			rc = OMRPORT_SIG_ERROR;
 		}
 	} else {
 		rc = registerMasterHandlers(portLibrary, flags, OMRPORT_SIG_FLAG_SIGALLASYNC);
@@ -488,7 +488,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary* portLibrary, omrsig_handl
 
 	if (0 != rc) {
 		Trc_PRT_signal_omrsig_set_async_signal_handler_exiting_did_nothing_possible_error(handler, handler_arg, flags);
-		return OMRPORT_SIG_ERROR;
+		return rc;
 	}
 	
 	omrthread_monitor_enter(asyncMonitor);
@@ -529,7 +529,7 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary* portLibrary, omrsig_handl
 																				OMR_GET_CALLSITE(),
 																				OMRMEM_CATEGORY_PORT_LIBRARY);
 			if (NULL == record) {
-				rc = 1;
+				rc = OMRPORT_SIG_ERROR;
 			} else {
 				record->portLib = portLibrary;
 				record->handler = handler;

From ae4a0844af7f217a50514c0fce611b8f7364cb20 Mon Sep 17 00:00:00 2001
From: Babneet Singh <sbabneet@ca.ibm.com>
Date: Thu, 3 May 2018 15:50:02 -0400
Subject: [PATCH 7/9] Cache info if a master handler is registered with a
 signal

omrsig_protect and omrsig_set_*_async_signal_handler may be invoked
multiple times. If a master handler is already registered with a signal,
then we don't want to re-register the master handler with that signal.

If a master handler is registered with a signal, then this information
is cached in signalsWithMasterHandlers. The port library signal bit is
set in signalsWithMasterHandlers.

When a user overrides the master handler by invoking
omrsig_register_os_handler, then the port library signal bit is un-set
in signalsWithMasterHandlers.

omrsig_set_single_async_signal_handler returns the oldOSHandler. The
oldOSHandler can be the master handler if the user re-invokes
omrsig_set_single_async_signal_handler for a signal. So, a user can
invoke omrsig_register_os_handler using a master handler. In this case,
we must set signalsWithMasterHandlers bits accordingly in
omrsig_register_os_handler.

This allows us to prevent OS calls to re-register master signal
handlers.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
---
 port/unix/omrsignal.c | 53 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 5 deletions(-)

diff --git a/port/unix/omrsignal.c b/port/unix/omrsignal.c
index 1091d9ce38e..18796fc004e 100644
--- a/port/unix/omrsignal.c
+++ b/port/unix/omrsignal.c
@@ -89,6 +89,15 @@ static struct {
  */
 static uint32_t signalsWithHandlers;
 
+/* Records the (port library defined) signals for which a master handler is
+ * registered. A master handler can be either masterSynchSignalHandler or
+ * masterASynchSignalHandler. A signal can only be associated to one master
+ * handler. If a master handler is already registered for a signal, then avoid
+ * re-registering a master handler for that signal. Access to this variable
+ * must be protected by the registerHandlerMonitor.
+ */
+static uint32_t signalsWithMasterHandlers;
+
 #if defined(OMR_PORT_ASYNC_HANDLER)
 static uint32_t shutDownASynchReporter;
 #endif
@@ -596,6 +605,23 @@ omrsig_register_os_handler(struct OMRPortLibrary *portLibrary, uint32_t portlibS
 	} else {
 		omrthread_monitor_enter(registerHandlerMonitor);
 		rc = registerSignalHandlerWithOS(portLibrary, portlibSignalFlag, (unix_sigaction)newOSHandler, oldOSHandler);
+		if (0 == rc) {
+			/*  A user-specified handler has been successfully registered for a signal. */
+			if ((newOSHandler == (void *)masterSynchSignalHandler)
+				|| (newOSHandler == (void *)masterASynchSignalHandler)
+			) {
+				/* User-specified handler is a master handler. So, set the portlibSignalFlag bit in
+				 * signalsWithMasterHandlers.
+				 */
+				signalsWithMasterHandlers |= portlibSignalFlag;
+			} else {
+				/* If the user-specified handler is not a master handler, then unset the
+				 * portlibSignalFlag bit in signalsWithMasterHandlers. This suggests that a
+				 * master handler is no longer registered with the portlibSignalFlag's signal.
+				 */
+				signalsWithMasterHandlers &= ~portlibSignalFlag;
+			}
+		}
 		omrthread_monitor_exit(registerHandlerMonitor);
 	}
 
@@ -1331,6 +1357,8 @@ addAsyncSignalsToSet(sigset_t *ss)
 
 /**
  * Registers the master handler for the signals in flags that don't have one.
+ * If signalsWithMasterHandlers suggests a master handler is already registered
+ * with a signal, then a master handler isn't registered again for that signal.
  *
  * Calls to this function must be synchronized using registerHandlerMonitor.
  *
@@ -1374,11 +1402,24 @@ registerMasterHandlers(OMRPortLibrary *portLibrary, uint32_t flags, uint32_t all
 		for (portSignalType = OMRPORT_SIG_SMALLEST_SIGNAL_FLAG; portSignalType < allowedSubsetOfFlags; portSignalType = portSignalType << 1) {
 			/* iterate through all the  signals and register the master handler for those that don't have one yet */
 
-			if (OMR_ARE_ANY_BITS_SET(flagsSignalsOnly, portSignalType)) {
-				/* we need a master handler for this (portSignalType's) signal */
+			if (OMR_ARE_ALL_BITS_SET(flagsSignalsOnly, portSignalType)) {
+				if (OMR_ARE_NO_BITS_SET(signalsWithMasterHandlers, portSignalType)) {
+					/* Register a master handler for this (portSignalType's) signal. */
+					if (0 != registerSignalHandlerWithOS(portLibrary, portSignalType, handler, oldOSHandler)) {
+						return OMRPORT_SIG_ERROR;
+					}
 
-				if (0 != registerSignalHandlerWithOS(portLibrary, portSignalType, handler, oldOSHandler)) {
-					return OMRPORT_SIG_ERROR;
+					/* After successfully registering the master handler, set the signal
+					 * bit in signalsWithMasterHandlers.
+					 */
+					signalsWithMasterHandlers |= portSignalType;
+				} else {
+					/* If the master handler is already registered, then the oldOSHandler must represent the
+					 * master handler.
+					 */
+					if (NULL != oldOSHandler) {
+						*oldOSHandler = (void *)handler;
+					}
 				}
 			}
 		}
@@ -1610,10 +1651,12 @@ sig_full_shutdown(struct OMRPortLibrary *portLibrary)
 		/* register the old actions we overwrote with our own */
 		for (index = 1; index < ARRAY_SIZE_SIGNALS; index++) {
 			if (oldActions[index].restore) {
+				uint32_t portlibSignalFlag = mapUnixSignalToPortLib(index, 0);
 				OMRSIG_SIGACTION(index, &oldActions[index].action, NULL);
 				/* record that we no longer have a handler installed with the OS for this signal */
 				Trc_PRT_signal_sig_full_shutdown_deregistered_handler_with_OS(portLibrary, index);
-				signalsWithHandlers &= ~mapUnixSignalToPortLib(index, 0);
+				signalsWithHandlers &= ~portlibSignalFlag;
+				signalsWithMasterHandlers &= ~portlibSignalFlag;
 				oldActions[index].restore = 0;
 			}
 		}

From dfb46a15f60fe7e8f6ee4da04554dc61b717b636 Mon Sep 17 00:00:00 2001
From: Babneet Singh <sbabneet@ca.ibm.com>
Date: Fri, 4 May 2018 17:10:45 -0400
Subject: [PATCH 8/9] Properly handle NULL values for oldOSHandler

omrsig_set_single_async_signal_handler and omrsig_register_os_handler
have been updated to properly handle NULL values for oldOSHandler.
Function description has been updated for both of them. A trace point
was accessing oldOSHandler without a NULL check. A NULL check has been
added before the trace point accesses oldOSHandler.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
---
 port/common/omrsignal.c | 13 ++++++++++---
 port/unix/omrsignal.c   | 13 +++++++++++--
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/port/common/omrsignal.c b/port/common/omrsignal.c
index daefeda235d..52740fc49f2 100644
--- a/port/common/omrsignal.c
+++ b/port/common/omrsignal.c
@@ -180,8 +180,11 @@ omrsig_set_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsig_handl
  * entry corresponding to omrsig_handler_fn is removed, and related resources are freed. portlibSignalFlag can only
  * have one signal flag set; otherwise, OMRPORT_SIG_ERROR is returned. One omrsig_handler_fn handler is registered
  * with a signal at any time instead of multiple handlers. When associating a new omrsig_handler_fn with a signal,
- * prior omrsig_handler_fn(s) are dissociated from the signal. The address of the old signal handler function is
- * stored in oldOSHandler. This function supports all signals listed in OMRPORT_SIG_FLAG_SIGALLASYNC.
+ * prior omrsig_handler_fn(s) are dissociated from the signal. This function supports all signals listed in
+ * OMRPORT_SIG_FLAG_SIGALLASYNC.
+ *
+ * The address of the old signal handler function is stored in *oldOSHandler. In case of error or if NULL is provided
+ * for oldOSHandler, then *oldOSHandler is not updated to reflect the old signal handler function.
  *
  * @param[in] portLibrary The port library
  * @param[in] handler the function to call if an asynchronous signal arrives
@@ -229,7 +232,11 @@ omrsig_map_portlib_signal_to_os_signal(struct OMRPortLibrary *portLibrary, uint3
  * @brief Register a handler with the OS. For an invalid portlibSignalFlag, an error is returned.
  * portlibSignalFlag is invalid: if it is zero or if multiple signal bits are specified or if the
  * specified flag is not supported. If OS fails to register newOSHandler for the specified signal,
- * then an error is returned. In error cases, oldOSHandler won't be updated.
+ * then an error is returned.
+ *
+ * The address of the old signal handler function is stored in *oldOSHandler. In case of error or
+ * if NULL is provided for oldOSHandler, then *oldOSHandler is not updated to reflect the old
+ * signal handler function.
  *
  * This function may override a master handler which was previously set by omrsig_protect or
  * omrsig_set_*_async_signal_handler variant. The records associated with the master handler
diff --git a/port/unix/omrsignal.c b/port/unix/omrsignal.c
index 18796fc004e..489868bc6a2 100644
--- a/port/unix/omrsignal.c
+++ b/port/unix/omrsignal.c
@@ -575,7 +575,12 @@ omrsig_set_single_async_signal_handler(struct OMRPortLibrary *portLibrary, omrsi
 
 	omrthread_monitor_exit(asyncMonitor);
 
-	Trc_PRT_signal_omrsig_set_single_async_signal_handler_exiting(rc, handler, handler_arg, portlibSignalFlag, *oldOSHandler);
+	if (NULL != oldOSHandler) {
+		Trc_PRT_signal_omrsig_set_single_async_signal_handler_exiting(rc, handler, handler_arg, portlibSignalFlag, *oldOSHandler);
+	} else {
+		Trc_PRT_signal_omrsig_set_single_async_signal_handler_exiting(rc, handler, handler_arg, portlibSignalFlag, NULL);
+	}
+
 	return rc;
 }
 
@@ -625,7 +630,11 @@ omrsig_register_os_handler(struct OMRPortLibrary *portLibrary, uint32_t portlibS
 		omrthread_monitor_exit(registerHandlerMonitor);
 	}
 
-	Trc_PRT_signal_omrsig_register_os_handler_exiting(rc, portlibSignalFlag, newOSHandler, *oldOSHandler);
+	if (NULL != oldOSHandler) {
+		Trc_PRT_signal_omrsig_register_os_handler_exiting(rc, portlibSignalFlag, newOSHandler, *oldOSHandler);
+	} else {
+		Trc_PRT_signal_omrsig_register_os_handler_exiting(rc, portlibSignalFlag, newOSHandler, NULL);
+	}
 
 	return rc;
 }

From 75f00a1c743a2689a2c9c371270853d8520be2ca Mon Sep 17 00:00:00 2001
From: Babneet Singh <sbabneet@ca.ibm.com>
Date: Thu, 3 May 2018 16:09:39 -0400
Subject: [PATCH 9/9] Remove unnecessary write barrier

At the end of registerSignalHandlerWithOS, a write barrier is issued
before updating signalsWithHandlers. According to the comment, the write
barrier is needed since registerHandlerMonitor is not acquired before
updating signalsWithHandlers at some places.

It has been confirmed that registerHandlerMonitor is acquired every time
signalsWithHandlers is accessed. So, the write barrier at the end of
registerSignalHandlerWithOS is no longer required.

In sig_full_shutdown, registerHandlerMonitor is not acquired before
updating signalsWithHandlers. This is an exception since no other code
should execute when OMR is shutting down.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
---
 port/unix/omrsignal.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/port/unix/omrsignal.c b/port/unix/omrsignal.c
index 489868bc6a2..355a51a10f0 100644
--- a/port/unix/omrsignal.c
+++ b/port/unix/omrsignal.c
@@ -1258,9 +1258,6 @@ registerSignalHandlerWithOS(OMRPortLibrary *portLibrary, uint32_t portLibrarySig
 		}
 	}
 
-	/* CMVC 96193 signalsWithHandlers is checked without acquiring the registerHandlerMonitor */
-	issueWriteBarrier();
-
 	/* Set the portLibrarySignalNo bit in signalsWithHandlers to record successful registration
 	 * of the handler. */
 	signalsWithHandlers |= portLibrarySignalNo;