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.

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

Use of signal handlers needs to be improved #15

Closed
FDelporte opened this issue Jun 12, 2020 · 7 comments · Fixed by #32
Closed

Use of signal handlers needs to be improved #15

FDelporte opened this issue Jun 12, 2020 · 7 comments · Fixed by #32
Assignees
Labels
bug Something isn't working

Comments

@FDelporte
Copy link
Member

As reported by Jim Darby:
If you're running as root (never a good idea) and using pigpio directly then pigpio steals all the signal handlers. The JVM doesn't like this as it uses them in running the JVM so it ends up with the program crashing. I'm working on this by getting pi4j-v2 to put back the signal handlers that the JVM uses.

@FDelporte FDelporte added the bug Something isn't working label Jun 12, 2020
@FDelporte FDelporte added this to the v2.0 (initial release) milestone Jun 12, 2020
@hackerjimbo
Copy link

As a first step I printed out the signal handled at the very start of Java_com_pi4j_library_pigpio_internal_PIGPIO_gpioInitialise to see what it was life before gpioInitialise is called. It looks like quite a lot.

Then I looked at the code in pigpio and found something very interesting. There is actually a compile-time #define, EMBEDDED_IN_VM, which explicitly turns off pigpio's use of signals!

Furthermore, there is a run-time flag as well. The code looks like:

#ifndef EMBEDDED_IN_VM
if (!(gpioCfg.internals & PI_CFG_NOSIGHANDLER))
sigSetHandler();
#endif

So, we can avoid the entire problem and use the OS-supplied pigpio by setting that flag!

This looks like the footprints of someone being here before. I'm going to guess that it's Python. Now all we have to do is to figure out how to set that flag.

@hackerjimbo
Copy link

Brace yourselves. Here's the fix (I hope). In the native C bindings we have the function Java_com_pi4j_library_pigpio_internal_PIGPIO_gpioInitialise. This contains the single line return gpioInitialise();.

Immediately before that insert gpioCfgSetInternals (gpioCfgGetInternals () | PI_CFG_NOSIGHANDLER);

I can't actually test this as I can't build an armv6 native library right now, but I strongly believe it should work.

savageautomate added a commit that referenced this issue Jul 13, 2020
…ignals and preventing signals from reaching the JVM.
@savageautomate
Copy link
Member

savageautomate commented Jul 13, 2020

@hackerjimbo ,

Here is the compiled JAR with the changes you suggested if you would like to test it out:

JNIEXPORT jint JNICALL Java_com_pi4j_library_pigpio_internal_PIGPIO_gpioInitialise
(JNIEnv *env, jclass class)
{
// SEE: https://github.com/Pi4J/pi4j-v2/issues/15
// By default, PIGPIO steals all the signal handlers. The JVM doesn't like this
// as it uses them in running the JVM so it ends up with the program crashing.
// The following should disable the signal handlers inside the PIGPIO library.
gpioCfgSetInternals (gpioCfgGetInternals () | PI_CFG_NOSIGHANDLER);
// perform initialization of the PIGPIO library
return gpioInitialise();
}

@savageautomate
Copy link
Member

@hackerjimbo ,

While applying the fix you suggested, I also came across these lines of code:

public int gpioInitialise() throws IOException {
int result = 0;
logger.trace("[INITIALIZE] -> STARTED");
if(!this.initialized) {
// disable socket and pipes interfaces
int rslt = PIGPIO.gpioCfgInterfaces(3);
// initialize the PiGpio native library
result = PIGPIO.gpioInitialise();
validateResult(result);
// creat a terminate handler for received signals
PiGpioSignalCallback terminateSignalHandler = signum -> {
logger.trace("------------ HERE");
PIGPIO.gpioTerminate();
System.exit(signum);
};
// apply our terminate handler to these default system signals
PIGPIO.gpioSetSignalFunc(PiGpioConst.SIGHUP, terminateSignalHandler);
PIGPIO.gpioSetSignalFunc(PiGpioConst.SIGINT, terminateSignalHandler);
PIGPIO.gpioSetSignalFunc(PiGpioConst.SIGQUIT, terminateSignalHandler);
PIGPIO.gpioSetSignalFunc(PiGpioConst.SIGKILL, terminateSignalHandler);
PIGPIO.gpioSetSignalFunc(PiGpioConst.SIGTERM, terminateSignalHandler);
PIGPIO.gpioSetSignalFunc(PiGpioConst.SIGSTOP, terminateSignalHandler);
PIGPIO.gpioSetSignalFunc(PiGpioConst.SIGTSTP, terminateSignalHandler);
// initialization successful
this.initialized = true;
logger.debug("[INITIALIZE] -- INITIALIZED SUCCESSFULLY");
}
else{
logger.warn("[INITIALIZE] -- ALREADY INITIALIZED");
}
logger.trace("[INITIALIZE] <- FINISHED");
return result;
}

It looks like I was originally registering signal handlers with PiGPIO to try and capture exit/terminate signals and then terminate the PiGPIO library as well as terminate the JVM application with System.exit(signum);. This must be here because I was failing to get the Java application to exit cleanly when PiGPIO was stealing/handling the signals. I suspect this code could now be removed. What do you think?

FDelporte added a commit that referenced this issue Jul 13, 2020
FIX: Issue #15; Prevent the PiGPIO library from internally handling s…
@savageautomate
Copy link
Member

REOPENING only to address the question above (#15 (comment)) about getting rid of Java-side signal handlers registered with PiGPIO. I suspect these can be removed now? Especially if the change @hackerjimbo suggested completely disables signals in PiGPIO.

@hackerjimbo
Copy link

hackerjimbo commented Jul 13, 2020

@savageautomate Those signal handlers can now never be called as we no longer intercept the signals with PiGPIO. Hopefully :-)

Note that the signals aren't disabled, they're just handled by the JVM. Hopefully the JVM shutdown code should call the handler you registered.

Actually, I just tried a control-c on a running program. I got the following series of errors:

^C[main] WARN com.pi4j.library.pigpio.impl.PiGpioNativeImpl - PIGPIO ERROR: PI_BAD_HANDLE; unknown handle
[main] WARN com.pi4j.library.pigpio.impl.PiGpioNativeImpl - PIGPIO ERROR: PI_BAD_HANDLE; unknown handle
[main] WARN com.pi4j.library.pigpio.impl.PiGpioNativeImpl - PIGPIO ERROR: PI_BAD_HANDLE; unknown handle
[main] WARN com.pi4j.library.pigpio.impl.PiGpioNativeImpl - PIGPIO ERROR: PI_BAD_HANDLE; unknown handle
[main] WARN com.pi4j.library.pigpio.impl.PiGpioNativeImpl - PIGPIO ERROR: PI_BAD_HANDLE; unknown handle
[main] WARN com.pi4j.library.pigpio.impl.PiGpioNativeImpl - PIGPIO ERROR: PI_BAD_HANDLE; unknown handle
[main] WARN com.pi4j.library.pigpio.impl.PiGpioNativeImpl - PIGPIO ERROR: PI_BAD_HANDLE; unknown handle
[main] WARN com.pi4j.library.pigpio.impl.PiGpioNativeImpl - PIGPIO ERROR: PI_BAD_HANDLE; unknown handle
[main] WARN com.pi4j.library.pigpio.impl.PiGpioNativeImpl - PIGPIO ERROR: PI_BAD_HANDLE; unknown handle

savageautomate added a commit that referenced this issue Jul 13, 2020
… from Pi4J Java code.

We now disable signals in PiGPIO and no longer need to register for callbacks to shutdown gracefully.
@savageautomate
Copy link
Member

Removed these signal callbacks and deployed update SNAPSHOT JARs.
CLOSED.

MEBoo pushed a commit to MEBoo/pi4j-v2-issue26 that referenced this issue Jul 1, 2022
…ng signals and preventing signals from reaching the JVM.
MEBoo pushed a commit to MEBoo/pi4j-v2-issue26 that referenced this issue Jul 1, 2022
…acks from Pi4J Java code.

We now disable signals in PiGPIO and no longer need to register for callbacks to shutdown gracefully.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants