Subject: bug#227: nbd-client: At shutdown nbd-client disabled before file-systems could be cleanly unmounted

From: Jesse Smith <[email-masked]>
Date: Sun, 19 Feb 2023 01:03:16 -0400

On 2023-02-16 12:17 p.m., Mark Hindley wrote:
> Control: reassign -1 sysvinit-utils
> Control: affects -1 nbd-client
>
> Jesse,
>
> Thanks for you quick reply.
>
> On Thu, Feb 16, 2023 at 11:54:32AM -0400, Jesse Smith wrote:
>> I see two possible ways we could fix this:
>>
>> 1. Create a command line flag which disables the SIGSTOP and SIGCONT
>> signals being sent. This is an easy fix, quick and dirty. The potential
>> downside is if someone disables the STOP signal then maybe processes
>> terminate, move groups, or are replaced before we get around to sending
>> them the KILL signal. This probably won't happen, but it means killall5
>> is working with a "moving target".
>>
>> 2. We can run SIGSTOP on all processes _except_ those in the omit list.
>> This will be a lot slower than the existing "kill(-1, SIGSTOP)" call we
>> currently make. But I think it's more correct.
>>
>> Basically the new work flow would look like this:
>>
>> 1. Send all processes except those omitted the SIGSTOP command.
>> 2. Send all processes except those omitted the SIGKILL command.
>> 3. Send all processes except those omitted the SIGCONT command.
>>
>> Option #2 is slow and ugly, but seems "correct" from a behaviour point
>> of view.
> I don't really like #1: it will mean patching callers to get the (more?) correct
> behaviour. So #2 seems better. Will it be significantly slower?. You could only
> use it when omitpid is specified? If there are no omitpids, the current
> behaviour seems fine.
>

I looked at this issue some more and it turns out option #2 doesn't
really makes sense with the way killall5 works. The SIGSTOP signal is
sent out to basically pause everything on the system so we can sort
through what is available to be terminated and what should be spared. If
we want to go through the whole process tree to see what should or
should not be stopped first, then it removes the point of sending
SIGSTOP to give us a fixed collection of processes.

In other words, sending SIGSTOP only makes sense if we later want to
freeze everything and then sort processes into what we will terminate
and what will be omitted. If we are going to go through all the
processes and send some the SIGSTOP signal and not others, then we might
as well just skip the STOP and move straight on to terminating
appropriate processes.

I've tried to work in a reasonable middle ground. Now, if no processes
are in the "omit" list, we stick to the original behaviour. We stop
everything, sort through which processes we will terminate, then send
everything the SIGCONT signal. Just like before.

However, if there are any processes in the "omit" list, then we err on
the side of caution. Nothing is sent STOP or CONT, we just sort through
the processes we know about quickly and try to terminate everything
we're supposed to kill. And ignore anything in the process list.

It's not perfect, we could conceivably miss a process that was created
while we were sorting through the list after the point we'd normally
send SIGSTOP. However, this approach makes certain we do not damage any
process that can't handle receiving SIGSTOP.

The downside is that a process could get created after killall5 is run
and before it is terminated. However, this is always going to be the
case when we allow some programs to be in the "omit" list. Any of the
omitted programs could spawn a new process, so that risk already
existed. This doesn't really make the situation (much) worse.

I've tested this change and it seems to work. I'm attaching a new copy
of killall5 for testing. It's also available on GitHub in the 3.07
branch of sysvinit.

Also, as a bonus, a bunch of the killall5 code didn't initialize
variables, seeming to assume a specific code path or that the compiler
would initialize variables to zero/NULL. Since this isn't always true,
I've initialized variables we might now use in unexpected order or which
should be NULL/false at the start of the program.

- Jesse