[arch-general] [PATCH 21/48] Both rc.single and rc.shutdown use the same code to kill everything.

[Victor Lowther]

On Wed, Sep 1, 2010 at 5:41 AM, Kurt J. Bosch
<kjb-temp-2009 at alpenjodel.de> wrote:
> 2010-08-31 13:16, Dave Reisner:
>>
>> On Tue, Aug 31, 2010 at 10:07:52AM +0200, Kurt J. Bosch wrote:
>>>
>>> --snip--
>>>
>>> I suggest:
>>>
>>>  From b202be97f8dc1c0c68aaea792d4457c674c673f3 Mon Sep 17 00:00:00 2001
>>> From: Kurt J. Bosch<kjb-temp-2009 at alpenjodel.de>
>>> Date: Tue, 31 Aug 2010 09:57:47 +0200
>>> Subject: [PATCH 17/17] Correct behaviour of kill_everything()
>>>
>>> ---
>>>  functions |   11 +++++------
>>>  1 files changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/functions b/functions
>>> index b9ba718..3ca7324 100644
>>> --- a/functions
>>> +++ b/functions
>>> @@ -205,10 +205,9 @@ ck_status() {
>>>  kill_everything() {
>>>      # $1 = where we are being called from.
>>>      # This is used to determine which hooks to run.
>>> -    # Find daemons NOT in the DAEMONS array. Shut these down first
>>> -    for daemon in /var/run/daemons/*; do
>>> -        [[ -f $daemon ]] || continue
>>> -        daemon=${daemon##*/}
>>> +    # Find daemons NOT in the DAEMONS array.
>>> +    # Shut these down first in reverse order.
>>> +    for daemon in $( /bin/ls -t /var/run/daemons ); do
>>>        in_array "$daemon" "${DAEMONS[@]}" || stop_daemon "$daemon"
>>>      done
>>>
>>> @@ -220,7 +219,7 @@ kill_everything() {
>>>
>>>        # Terminate all processes
>>>      stat_busy "Sending SIGTERM To Processes"
>>> -    run_hook "$1_prekillall"
>>> +    run_hook "${1}_prekillall"
>>>      /sbin/killall5 -15&>  /dev/null
>>>      /bin/sleep 5
>>>      stat_done
>>> @@ -230,7 +229,7 @@ kill_everything() {
>>>      /bin/sleep 1
>>>      stat_done
>>>
>>> -    run_hook "$1_postkillall"
>>> +    run_hook "${1}_postkillall"
>>>  }
>>>
>>>  activate_vgs() {
>>> --
>>> 1.7.0.3
>>>
>>
>> Parsing the output of ls will never be better than using shell globbing
>> no matter how much simpler it might make the code appear to be. Not only
>> are you avoiding a fork, but the shell glob will properly handle any odd
>> characters thrown into the mix. You'll see breakage on something as
>> simple as a space in your suggestion. While I'm inclined to believe that
>> there will never be a space in the name of a daemon in Arch, if we're
>> going for pure Bash in this rewrite, let's use Bash instead of
>> mindlessly forking.
>>
> NAK. The patch just reverts breaking of the current behaviour which is
> shutdown daemons in reverse order of start. AFAIK bash globbing is unable to
> sort on timestamps, right?

Bash globbing cannot handle it, but bash knows how to test file
timestamps, so we could code something up that handles killing the
daemons in reverse timestamp order if we had to. However, there is no
reason to think that the order is significant for daemons not in
$DAEMONS.  If there are any dependencies that this code does not
handle gracefully, the offending daemons will be killed when we kill
-15 and then kill -9 the rest of the processes.  If a daemon does not
handle SIGTERM gracefully because of dependency issues, it has other,
more significant problems.