Signal processing in user application not possible (c application)

[adrianmiriuta]

1. pigpio does not handle meaningfully most signals
2. it also blocks user handling of those signals by catching them and exiting.

     DBG(DBG_ALWAYS, "Unhandled signal %d, terminating\n", signum);
     gpioTerminate();
     exit(-1);

3. it also catches SIGHUP and SIGKILL and closes the gpiod daemon,
   while user app cannot terminate,
   This signals should be handled by the main app, not the library.

      gpioSetSignalFunc(SIGHUP, terminate);
      gpioSetSignalFunc(SIGTERM, terminate);  

[guymcswain]

You are aware of this from the doc:

If you intend to rely on signals sent to your application, you should turn off the internal signal handling as shown in this example:

int cfg = gpioCfgGetInternals();
cfg |= PI_CFG_NOSIGHANDLER;  // (1<<10)
gpioCfgSetInternals(cfg);
int status = gpioInitialise();

[adrianmiriuta]

@guymcswain

1. no I was not aware of that .
2. it does not work.

// gpio init
int cfg = gpioCfgGetInternals();
cfg |= PI_CFG_NOSIGHANDLER;  // (1<<10)
gpioCfgSetInternals(cfg);

if (gpioInitialise() < 0) {
  syslog(LOG_ERR, "ERROR: gpioInitialise (%s,%d)", __FILE__, __LINE__);
  exit(EXIT_FAILURE);
}

log

Apr  3 20:59:31 pi systemd[1]: Started mpp-comtech.
Apr  3 20:59:32 pi mpp-comtech[10453]: ERROR: gpioInitialise (src/mpp-comtech.cpp,210)
Apr  3 20:59:32 pi systemd[1]: mpp-comtech.service: Main process exited, code=exited, status=1/FAILURE
Apr  3 20:59:32 pi systemd[1]: mpp-comtech.service: Failed with result 'exit-code'.
Apr  3 20:59:34 pi systemd[1]: Stopped mpp-comtech.

addition

1. I rebooted after reinstalling original libgpio and managed to initalize.
2. SIGTERM AND SIGKILL are not available to application.

[berndporr]

@adrianmiriuta that works for me without any problems:
https://github.com/berndporr/alphabot/blob/main/alphabot.cpp#L15
I suspect that your gpioInitialise fails for other reasons not related to signals.

However, I agree that managing all signals should be rather on option to be switched on than off but that would of course now break the behaviour of the library. I also tripped over this as I didn't expect it would catch all signals.

@guymcswain, perhaps a solution would be that the signal handler in pigpio really just handles the signals which are registered via the API after a callback handler has been registered and leaves the other ones alone? Also because just now a non-handled signal causes a brutal exit(-1) which made my robot actually crash into a wall because the program terminated instantly. It would be much better in that case just to ignore these signals and let them dealt with the main program in a transparent way. The exit(-1) is surely not a feature which needs to be kept?

[berndporr]

Here is my suggestion to get rid of the exit(-1) issue:
Instead of setting all signal handlers here in a loop:
https://github.com/joan2937/pigpio/blob/master/pigpio.c#L5646
they could be just set when the callback is set:
https://github.com/joan2937/pigpio/blob/master/pigpio.c#L12844
https://github.com/joan2937/pigpio/blob/master/pigpio.c#L12864
Essentially moving

memset(&new, 0, sizeof(new));
new.sa_handler = sigHandler;
sigaction(signum, &new, NULL);

into these two functions so that only those handlers are registered which actually have valid callbacks. Let me know what you think?

[berndporr]

Should add that if the user provides NULL for the callback it can be set back to signal(signo, SIG_IGN);.

[adrianmiriuta]

@berndporr

1. I think pigpio should only catch an handle the signals it really uses
2. the user defined signals ( like gpioSetSignalFunc(SIGTSTP, signalHandler); )
   are user responsability
3. all others should be ignored
4. as a quick and dirty fix I commented out SIGKILL and SIGTERM exits in pigpio .
   but they need serious rewriting.
5. the init failures are due to some other problem of pigpio,
   after a reboot i didn't got them any more.

[guymcswain]

I think I'm also not an expert in this area but I'll share my thoughts and observations:

• The daemon invokes the library, stubs out the stdin/stdout streams but needs to handle signals like SIGTERM, SIGKILL.
• Applications invoking the library spawn processes similar to the daemon, ie, socket and fifo threads enabling remote/local API access.
• Users of the library at the C level want total control over signals but from the co-dependencies between the library and the daemon restrictions were imposed.
• For me to unwind these two would require an enormous amount of time to make sure I didn't break anything, which is my utmost priority.
• Therefore, I need a really good justification for doing this or we should appeal to the author of pigpio for help.

[adrianmiriuta]

@guymcswain

1. the library does not need to handle SIGKILL and SIGTERM
   it is user responsability to gracefully terminate (gpioTerminate();)
2. I also think it's not a trivial task and demands deep knowledge of the library (which I do not posess)
3. This is a task for the whole comunity (but surely ... Help from the Creator would be the best)

[berndporr]

I see where you are coming from in terms of demon and C level. I think it would be good to talk to the author of pigpio and at least get rid of the exit(-1) issue by just registering signals which are really needed? I think that could be dealt with completely transparently without changing anything (except of not bailing out with exit(-1) )?
https://github.com/joan2937/pigpio/blob/master/pigpio.c#L5630

[adrianmiriuta]

@berndporr
there are two of them, it also exits here

pigpio/pigpio.c

Line 5640 in c33738a

exit(-1);

[guymcswain]

@berndporr , I'm willing to take small steps to make improvements to the lib. @adrianmiriuta , it sounds to me that you are not acceptable even with these changes. Is that the case?

it is user responsability to gracefully terminate (gpioTerminate();)

Please also consider that users don't always do things correctly and that you can leave the system in a horrific state if you don't exit gracefully. I think this may have been an intent from the author. I'm certainly biased to avoiding issues that I have to deal with later as the maintainer of pigpio.

[adrianmiriuta]

@guymcswain
sorry, I did not say that ...

1. Any change that makes the library behaviour more predictable and well behaved is acceptable
   as long as It does not break the API. (therehin lies the problem ...)
2. You are correct ... I wager there are a lot of apps that wil break if you change the SIGKILL SIGTERM behaviour
   because users do not terminate themselves the library tasks , so the daemon will become a orphaned process.
3. As I said It's not a trivial task to handle.

[guymcswain]

@berndporr ,

Also because just now a non-handled signal causes a brutal exit(-1) which made my robot actually crash into a wall because the program terminated instantly.

Firstly, sorry your robot crashed. Can you explain why you were not able to handle the signal after setting bit 10 to the internals configuration register?

[berndporr]

That was before I asked here how to disable signal handling. It works well now.
https://github.com/berndporr/alphabot/blob/main/alphabot.cpp#L15

It's rather for other C coders who use signals in their code and then caught by surprise when it exits. Plus in most cases then the setting of the flag to disable signal handling won't be needed so will cause less trouble for both the maintainers and the users. Basically: it's not about me (I can live with it) but making the library as easy to use as possible - so community care!

[berndporr]

I could create a pull request which gets rid of the exit(-1) and only registers the signal handlers which are actually in use. Initially only the ones which deal with the debug level and then the others are registered when a callback has been set for the corresponding signal.

[guymcswain]

Go ahead with a PR so I can review exactly the scope of the changes you are proposing.

[joan2937]

The reason signals work the way they do was to ensure the DMA engine was shut down on program failure. I was worried the DMA engine would trash all of user memory and corrupt SD cards. That paranoia led me to exit on any unhandled signal. Nowadays I'd probably take a more relaxed approach and says tevs.

[berndporr]

I see the point that the DMA needs to be stopped at program failure as it's quite a scary mechanism. I agree. I guess that makes a case for catching signals related to a program crash / shutdown and being paranoid here is a good thing. Though the bog standard user & realtime signals are not triggered during shutdown so no need to trigger an exit(-1) here. I'll have a go and create a pull request and take that into account. In this way also less users are forced to switch off the signal handling completely.