Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JVM Signal Handling Support [Part 2] #1832

Merged
merged 5 commits into from
May 17, 2018
+64 −58
Merged

JVM Signal Handling Support [Part 2] #1832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b0d7840
Add new function registerOSHandler
babsingh Apr 25, 2018
d6833aa
Fix comments and formatting
babsingh Apr 25, 2018
fdd18bf
Use registerOSHandler in JVM_RegisterSignal
babsingh May 3, 2018
3008dd5
Fix usage of omrsig_set_async_signal_handler
babsingh Apr 26, 2018
ab9b544
Fix code to remove predefinedHandlerWrapper
babsingh May 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions runtime/harmony/include/hyport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2001, 2017 IBM Corp. and others
* Copyright (c) 2001, 2018 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -509,7 +509,7 @@ typedef struct HyPortLibrary
I_32 (*sig_can_protect) (struct HyPortLibrary * portLibrary,
U_32 flags);
/** see @ref hysig.c::hysig_set_async_signal_handler "hysig_set_async_signal_handler"*/
U_32 (*sig_set_async_signal_handler) (struct HyPortLibrary *
I_32 (*sig_set_async_signal_handler) (struct HyPortLibrary *
portLibrary,
hysig_handler_fn handler,
void *handler_arg,
Expand Down
4 changes: 2 additions & 2 deletions runtime/harmony/stubs/hyport_shim.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2001, 2014 IBM Corp. and others
* Copyright (c) 2001, 2018 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -990,7 +990,7 @@ hystub_sig_can_protect(struct HyPortLibrary *portLibrary, U_32 flags)
return j9sig_can_protect(flags);
}

U_32
I_32
hystub_sig_set_async_signal_handler(struct HyPortLibrary *portLibrary, hysig_handler_fn handler, void *handler_arg, U_32 flags)
{
PORT_ACCESS_FROM_PORT((J9PortLibrary *)portLibrary->self_handle);
Expand Down
45 changes: 6 additions & 39 deletions runtime/j9vm/jvm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4618,14 +4618,6 @@ JVM_RegisterSignal(jint sigNum, void *handler)
J9VMThread *currentThread = vmFuncs->currentVMThread(javaVM);
BOOLEAN isShutdownSignal = isSignalUsedForShutdown(sigNum);

#if !defined(WIN32)
struct sigaction newSignalAction;
struct sigaction oldSignalAction;

memset(&newSignalAction, 0, sizeof(struct sigaction));
memset(&oldSignalAction, 0, sizeof(struct sigaction));
#endif /* !defined(WIN32) */

Trc_SC_RegisterSignal_Entry(currentThread, sigNum, handler);

if (isSignalReservedByJVM(sigNum)) {
Expand All @@ -4645,39 +4637,14 @@ JVM_RegisterSignal(jint sigNum, void *handler)
goto exit;
} else {
/* Register the signal. */
IDATA isHandlerRegistered = 0;
if ((void *)J9_PRE_DEFINED_HANDLER_CHECK == handler) {
if (0 != vmFuncs->registerPredefinedHandler(javaVM, sigNum, &oldHandler)) {
Trc_SC_RegisterSignal_FailedToRegisterHandler(currentThread, sigNum, handler, oldHandler);
}
isHandlerRegistered = vmFuncs->registerPredefinedHandler(javaVM, sigNum, &oldHandler);
} else {
#if defined(WIN32)
oldHandler = (void *)OMRSIG_SIGNAL(sigNum, handler);
#else /* defined(WIN32) */
/* Don't block any signals. */
if (0 != sigemptyset(&newSignalAction.sa_mask)) {
/* oldHandler is already initialized to J9_SIG_ERR. */
goto exit;
}

/* SA_RESTART - make certain system calls restartable across signals.
* SA_NODEFER - do not prevent the signal from being received from
* within its own signal handler.
* SA_SIGINFO - sa_sigaction specifies the signal handling function.
*/
newSignalAction.sa_flags = SA_RESTART | SA_SIGINFO | SA_NODEFER;

#if defined(S390) && defined(LINUX)
newSignalAction.sa_sigaction = (void *)handler;
#else /* defined(S390) && defined(LINUX) */
newSignalAction.sa_sigaction = (void (*)(int, siginfo_t *, void *))handler;
#endif /* defined(S390) && defined(LINUX) */

if (0 == OMRSIG_SIGACTION(sigNum, &newSignalAction, &oldSignalAction)) {
oldHandler = (void *)oldSignalAction.sa_sigaction;
} else {
Trc_SC_RegisterSignal_FailedToRegisterHandler(currentThread, sigNum, handler, oldHandler);
}
#endif /* defined(WIN32) */
isHandlerRegistered = vmFuncs->registerOSHandler(javaVM, sigNum, handler, &oldHandler);
}
if (0 != isHandlerRegistered) {
Trc_SC_RegisterSignal_FailedToRegisterHandler(currentThread, sigNum, handler, oldHandler);
}
}

Expand Down
1 change: 1 addition & 0 deletions runtime/oti/j9nonbuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4817,6 +4817,7 @@ typedef struct J9InternalVMFunctions {
void ( *flushProcessWriteBuffers)(struct J9JavaVM *vm);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI_USES_FLUSH */
IDATA ( *registerPredefinedHandler)(struct J9JavaVM *vm, U_32 signal, void **oldOSHandler);
IDATA ( *registerOSHandler)(struct J9JavaVM *vm, U_32 signal, void *newOSHandler, void **oldOSHandler);
} J9InternalVMFunctions;


Expand Down
1 change: 1 addition & 0 deletions runtime/oti/j9port_generated.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,7 @@ extern J9_CFUNC int32_t j9port_isCompatible(struct J9PortLibraryVersion *expecte
#define j9sig_set_single_async_signal_handler(param1,param2,param3,param4) OMRPORT_FROM_J9PORT(privatePortLibrary)->sig_set_single_async_signal_handler((OMRPortLibrary*)privatePortLibrary,(omrsig_handler_fn)param1,param2,param3,param4)
#define j9sig_map_os_signal_to_portlib_signal(param1) OMRPORT_FROM_J9PORT(privatePortLibrary)->sig_map_os_signal_to_portlib_signal((OMRPortLibrary*)privatePortLibrary,param1)
#define j9sig_map_portlib_signal_to_os_signal(param1) OMRPORT_FROM_J9PORT(privatePortLibrary)->sig_map_portlib_signal_to_os_signal((OMRPortLibrary*)privatePortLibrary,param1)
#define j9sig_register_os_handler(param1,param2,param3) OMRPORT_FROM_J9PORT(privatePortLibrary)->sig_register_os_handler((OMRPortLibrary*)privatePortLibrary,param1,(void *)param2,param3)
#define j9sig_info(param1,param2,param3,param4,param5) OMRPORT_FROM_J9PORT(privatePortLibrary)->sig_info(OMRPORT_FROM_J9PORT(privatePortLibrary),param1,param2,param3,param4,param5)
#define j9sig_info_count(param1,param2) OMRPORT_FROM_J9PORT(privatePortLibrary)->sig_info_count(OMRPORT_FROM_J9PORT(privatePortLibrary),param1,param2)
#define j9sig_set_options(param1) OMRPORT_FROM_J9PORT(privatePortLibrary)->sig_set_options(OMRPORT_FROM_J9PORT(privatePortLibrary),param1)
Expand Down
19 changes: 16 additions & 3 deletions runtime/oti/vm_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1584,15 +1584,28 @@ setBootLoaderModulePatchPaths(J9JavaVM * javaVM, J9Module * j9module, const char
* @brief Register jvminit.c::predefinedHandlerWrapper using j9sig_set_*async_signal_handler
* for the specified signal
*
* @param vm pointer to a J9JavaVM
* @param signal integer value of the signal
* @param oldOSHandler points to the old signal handler function
* @param[in] vm pointer to a J9JavaVM
* @param[in] signal integer value of the signal
* @param[out] oldOSHandler points to the old signal handler function
*
* @return 0 on success and non-zero on failure
*/
IDATA
registerPredefinedHandler(J9JavaVM *vm, U_32 signal, void **oldOSHandler);

/**
* @brief Register a signal handler function with the OS for the specified signal.
*
* @param[in] vm pointer to a J9JavaVM
* @param[in] signal integer value of the signal
* @param[in] newOSHandler address to the new signal handler function which will be registered
* @param[out] oldOSHandler points to the old signal handler function
*
* @return 0 on success and non-zero on failure
*/
IDATA
registerOSHandler(J9JavaVM *vm, U_32 signal, void *newOSHandler, void **oldOSHandler);

/* ---------------- romutil.c ---------------- */

/**
Expand Down
4 changes: 2 additions & 2 deletions runtime/tests/port/j9processTest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 1991, 2017 IBM Corp. and others
* Copyright (c) 1991, 2018 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -290,7 +290,7 @@ setTestProcessGroupSignalHandler(struct J9PortLibrary *portLibrary)
rc = 0;
}
#else /* defined(WIN32) */
U_32 setAsyncRC = J9PORT_SIG_ERROR;
I_32 setAsyncRC = J9PORT_SIG_ERROR;
setAsyncRC = j9sig_set_async_signal_handler(processGroupSignalHandler, NULL, J9PORT_SIG_FLAG_SIGQUIT);
if (J9PORT_SIG_ERROR == setAsyncRC) {
rc = -1;
Expand Down
1 change: 1 addition & 0 deletions runtime/vm/intfunc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,4 +362,5 @@ J9InternalVMFunctions J9InternalFunctions = {
flushProcessWriteBuffers,
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI_USES_FLUSH */
registerPredefinedHandler,
registerOSHandler,
};
2 changes: 2 additions & 0 deletions runtime/vm/j9vm.tdf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,3 +770,5 @@ TraceException=Trc_VM_failedtoAllocateGuardPage noEnv Overhead=1 Level=1 Templat

TraceException=Trc_VM_registerPredefinedHandler_invalidPortlibSignalFlag NoEnv Overhead=1 Level=1 Template="registerPredefinedHandler - invalid portlib signal flag: %zu"
TraceEvent=Trc_VM_signalDispatch_signalValue Overhead=1 Level=3 Template="signalDispatch - signal value: %d"

TraceException=Trc_VM_registerOSHandler_invalidPortlibSignalFlag NoEnv Overhead=1 Level=1 Template="registerOSHandler - invalid portlib signal flag: %zu"
41 changes: 31 additions & 10 deletions runtime/vm/jvminit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ freeJavaVM(J9JavaVM * vm)
#endif

/* Remove the predefinedHandlerWrapper. */
j9sig_set_async_signal_handler(predefinedHandlerWrapper, vm, 0);
j9sig_set_single_async_signal_handler(predefinedHandlerWrapper, vm, 0, NULL);

/* Unload before trace engine exits */
UT_MODULE_UNLOADED(J9_UTINTERFACE_FROM_VM(vm));
Expand Down Expand Up @@ -6231,13 +6231,14 @@ shutDownHookWrapper(struct J9PortLibrary* portLibrary, U_32 gpType, void* gpInfo
* Invoke jdk.internal.misc.Signal.dispatch(int number) in Java 9 and
* onwards. Invoke sun.misc.Signal.dispatch(int number) in Java 8.
*
* @param vmThread pointer to a J9VMThread
* @param signal integer value of the signal
* @param[in] vmThread pointer to a J9VMThread
* @param[in] signal integer value of the signal
*
* @return void
*/
static void
signalDispatch(J9VMThread *vmThread, I_32 signal) {
signalDispatch(J9VMThread *vmThread, I_32 signal)
{
J9JavaVM *vm = vmThread->javaVM;
J9NameAndSignature nas = {0};
I_32 args[] = {signal};
Expand Down Expand Up @@ -6265,16 +6266,17 @@ signalDispatch(J9VMThread *vmThread, I_32 signal) {
* in omrsignal.c once registered using j9sig_set_*async_signal_handler
* for a specific signal.
*
* @param portLibrary the port library
* @param gpType port library signal flag
* @param gpInfo GPF information (will be NULL in this case)
* @param userData user data (will be a pointer to J9JavaVM in this case)
* @param[in] portLibrary the port library
* @param[in] gpType port library signal flag
* @param[in] gpInfo GPF information (will be NULL in this case)
* @param[in] userData user data (will be a pointer to J9JavaVM in this case)
*
* @return 0 on success and non-zero on failure
*
*/
static UDATA
predefinedHandlerWrapper(struct J9PortLibrary *portLibrary, U_32 gpType, void *gpInfo, void *userData) {
predefinedHandlerWrapper(struct J9PortLibrary *portLibrary, U_32 gpType, void *gpInfo, void *userData)
{
J9JavaVM *vm = (J9JavaVM *)userData;
J9JavaVMAttachArgs attachArgs = {0};
J9VMThread *vmThread = NULL;
Expand Down Expand Up @@ -6323,7 +6325,8 @@ predefinedHandlerWrapper(struct J9PortLibrary *portLibrary, U_32 gpType, void *g
}

IDATA
registerPredefinedHandler(J9JavaVM *vm, U_32 signal, void **oldOSHandler) {
registerPredefinedHandler(J9JavaVM *vm, U_32 signal, void **oldOSHandler)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need two copies of this code? Could this one call registerOSHandler with predefinedHandlerWrapper as the newOSHandler? Or can we push knowledge of which handler to pass to the caller?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

registerPredefinedHandler sets OS handler as omrsignal.c::masterASynchSignalHandler [not exposed externally], and adds predefinedHandlerWrapper to the asyncHandlerList list. predefinedHandlerWrapper is not an OS handler.

registerOSHandler registers the user-provided OS handler with specified signal.

we can merge registerOSHandler and registerPredefinedHandler. use newOSHandler as J9_PRE_DEFINED_HANDLER_CHECK(2) to specify the predefinedHandlerWrapper. should i proceed with this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. The two functions are subtly different. Better to leave them as two different calls.

{
IDATA rc = 0;
U_32 portlibSignalFlag = 0;

Expand All @@ -6339,6 +6342,24 @@ registerPredefinedHandler(J9JavaVM *vm, U_32 signal, void **oldOSHandler) {
return rc;
}

IDATA
registerOSHandler(J9JavaVM *vm, U_32 signal, void *newOSHandler, void **oldOSHandler)
{
IDATA rc = 0;
U_32 portlibSignalFlag = 0;

PORT_ACCESS_FROM_JAVAVM(vm);

portlibSignalFlag = j9sig_map_os_signal_to_portlib_signal(signal);
if (0 != portlibSignalFlag) {
rc = j9sig_register_os_handler(portlibSignalFlag, newOSHandler, oldOSHandler);
} else {
Trc_VM_registerOSHandler_invalidPortlibSignalFlag(portlibSignalFlag);
}

return rc;
}

static jint
initializeDDR(J9JavaVM * vm)
{
Expand Down