On Sat, Apr 10, 2021 at 11:11:05PM +0200, Lorenzo wrote: Hi,
> > 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? Nothing specific; I was basing my recommendation on the following: * this code and documentation has been around for a long time; * this part of the code is relatively rarely used; * there are likely setups that were created on the assumption that the documentation was correct, but not tested extensively (I know I'm guilty of this). (I have, of course, no idea how many setups exist that rely on the existing but undocumented behaviour.) Additionally, the documentation as it is now is clear and straightforward, with few special cases; while the actual behaviour of runsv, while I agree it can make sense, is more difficult to explain clearly. In the case of runsv, I suppose I see the manpage as a specification for, not an explanation of, what the code does. All of these arguments are weak. The main point is that the documentation should accurately reflect what the code does; I realize that fixing the documentation is easier. I also see the merit in your arguments. > > 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? Sure. In https://serverfault.com/questions/612458/runit-does-not-kill-process-on-sv-stop-or-sv-reload/625822, someone was looking for a way to stop child processes of a runit-supervised service on service shutdown. I suggested a custom 'd' script, and "Keith" pointed out the discrepancy between the documentation and the code in a his comment https://serverfault.com/questions/612458/runit-does-not-kill-process-on-sv-stop-or-sv-reload/625822#comment1375192_625822. (It's also true that in this specific case, the poster would need a 't' script in addition to the 'd' script even if the documentation were correct; otherwise 'sv t' would still leave his sub-processes around.) Best regards, András -- To whom the gods destroy, they first teach Windows...
