Control: tag -1 moreinfo
Control: tag -1 confirmed

On Sun, 28 Feb 2021 22:22:20 +0100
Andras Korn <korn-debb...@elan.rulez.org> wrote:

> Hi,
> 

Hello,

[Sorry, very late reply]
thanks for the report and the patch

> runsv(8) says:
> 
> CUSTOMIZE CONTROL
>        For each control character c sent to the control pipe, runsv
> first checks if service/control/c exists and is executable. If so, it
>        starts service/control/c and waits for it to terminate, before
>        interpreting the command. If the program exits with return
> code 0, runsv refrains from sending the service the corresponding
> signal. The command o is always considered as command u. On command d
> first service/control/t is checked, and then service/control/d. On
> command x first service/control/t is checked, and then
> service/control/x. The control of the optional log service cannot be
> customized.
> 
> This is, however, not what the code actually does.
> 
> [ ... ]
> 
> i.e. the control/d or control/x script is only run after runsv sent
> its child a SIGTERM, not before as the manpage suggests.

Right. Also it looks that with control/[dx] an override on the CONT
signal is disregarded. It's reasonable but it should be documented too.

> 
> I would suggest that the code should be changed to reflect the
> documented behaviour, not vice versa.

Ideally Gerrit Pape should make a call on this, but it's not
happening, so:
Could you elaborate more on why you want to update the code instead of
fixing the manpage?

If you add an override for the TERM signal it would be run before [dx],
so what you want is a way to do a custom action that is not run when
the service receives a plain TERM signal but is run only when runsv is
told to set the service down or to exit.
Do you have an example/use case for this?

On the other hand, with the current code we have a custom action that
is run after runsv has sent both TERM and CONT, looks like a special
case of finish file (?), not sure what can be a reasonable use case here
too..


>  Perhaps the following code
> would work (but I have not tested it):
> 
> void stopservice(struct svdir *s) {
>   if (s->pid) {
>     if (s->want == W_DOWN) {
>       kill(s->pid, SIGCONT);
>       if (custom(s, 'd')) return;
>     }
>     if (s->want == W_EXIT) {
>       kill(s->pid, SIGCONT);
>       if (custom(s, 'x')) return;
>     }
>     if (! custom(s, 't')) {
>       kill(s->pid, SIGTERM);
>       s->ctrl |=C_TERM;
>       update_status(s);
>     }
>   }
> }
> 

I'm not an expert in runit code and I have just had a quick look at
runsv code, but unless I'm overlooking something it seems to me
that with your patch we will get CONT, then TERM while the man page
says TERM, then CONT. Also, it's changing the behavior even when there
is no signal override at all.

> Credits to https://serverfault.com/users/135556/keith for pointing
> the bug out.

I can only see some info about the reporter profile, do you have a link
where I can read what was the reporter complaining of?

> 
> Best regards,
> 
> AndrĂ¡s

Regards,
Lorenzo

Reply via email to