BunsenLabs / bunsen-utilities

https://pkg.bunsenlabs.org/debian/pool/main/b/bunsen-utilities/
GNU General Public License v3.0
31 stars 21 forks source link

bl-tint2-restart needs to be stricter #15

Closed johnraff closed 9 years ago

johnraff commented 9 years ago

With default tint2 processes (no -c argument) bl-tint2-restart does not kill all, and duplicate processes are perpetuated.

john@bunsen:~$ pgrep -a tint2
8151 /bin/bash /usr/bin/bl-tint2-session
8260 tint2
9068 tint2
9086 tint2

After Settings>Tint2>Restart tint2, one more process has been added:

john@bunsen:~$ pgrep -a tint2
8151 /bin/bash /usr/bin/bl-tint2-session
8260 tint2
9176 tint2
9178 tint2
9180 tint2

After running the suggested new restart script:

john@bunsen:~$ pgrep -a tint2
9330 tint2

@capn-damo would it be OK to substitute this harsher restart script? It uses the KILL signal to make sure existing processes are really stopped, and only starts one instance of processes that have the same command line.

#!/bin/bash
#
# bl-tint2restart to be used by bl-tint2-pipemenu
# Written for BunsenLabs Linux by <damo> June 2015

declare -A commands # associative array
while read pid cmd; do
    if [[ ${cmd%% *} = tint2 ]]; then
        kill -KILL "$pid"
        commands[$cmd]=1 # duplicate commands will be launched only once
    fi
done <<< "$(pgrep -a tint2)"

sleep 1
for i in "${!commands[@]}" # go through the indexes
do
    (setsid $i &)
    sleep 0.1
done

sleep 1
bl-compositor --restart  # restart compositor

exit 0
capn-damo commented 9 years ago

By all means. You know this stuff better than me! (And add your name to the header info, since you have contributed a lot to the script ;) )

ghost commented 9 years ago

@johnraff Why is SIGKILL necessary? Does the program ignore SIGTERM? If need be, send SIGTERM, wait, check again and then send SIGKILL.

johnraff commented 9 years ago

SIGKILL should not be necessary, but the tint2 version in Jessie seems to have a freezing bug, and when it occurs SIGKILL is the only signal that will kill it. http://crunchbang.org/forums/viewtopic.php?pid=430787#p430787 Yes the script could be enhanced to try SIGTERM first. It's a bit of extra code, but I'll try it today.

In fact, the script could fairly easily be expanded to take any process name as argument to make it a generic bl-restart for possible use elsewhere. It would mean either modifying the call in bl-tint2-pipemenu or making a wrapper script bl-tint2-restart which called bl-restart tint2.

johnraff commented 9 years ago

@2ion I guess a 0.5s wait before using SIGKILL should be enough? something like:

declare -A commands # associative array
while read pid cmd; do
    if [[ ${cmd%% *} = tint2 ]]; then
        kill "$pid"
        sleep 0.5
        kill -0 "$pid" && kill -KILL "$pid"
        commands[$cmd]=1 # duplicate commands will be launched only once
    fi
done <<< "$(pgrep -a tint2)"

Although really I'm not sure if there's any meaning in testing with kill -0 in kill -0 "$pid" && kill -KILL "$pid". Just kill -KILL "$pid" >/dev/null 2>&1 after the sleep would be enough, no?

johnraff commented 9 years ago

I've thought some more, and there's a possible race condition with the above code. During the sleep 0.5 it's possible, even if unlikely, that another process would start with the same $pid that the killed tint2 process had. That innocent process would then be killed.

It can be avoided by re-running the loop using the output of pgrep -a tint2. Thus:

#!/bin/bash
#
# bl-tint2restart to be used by bl-tint2-pipemenu
# Written for BunsenLabs Linux by <damo> June 2015
# and <johnraff> July 2015

declare -A commands # associative array

while read pid cmd; do
    if [[ ${cmd%% *} = tint2 ]]; then
        kill "$pid"
        commands[$cmd]=1 # duplicate commands will be launched only once
    fi
done <<< "$(pgrep -a tint2)"

sleep 1

# any processes still running will be killed with SIGKILL
while read pid cmd; do
    if [[ ${cmd%% *} = tint2 ]]; then
        kill -KILL "$pid"
        commands[$cmd]=1
    fi
done <<< "$(pgrep -a tint2)"

sleep 1
for i in "${!commands[@]}" # go through the indexes
do
    (setsid $i &)
    sleep 0.1
done

sleep 1
bl-compositor --restart  # restart compositor

exit 0