Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bl-tint2-restart needs to be stricter #15

Closed
johnraff opened this issue Jul 6, 2015 · 5 comments
Closed

bl-tint2-restart needs to be stricter #15

johnraff opened this issue Jul 6, 2015 · 5 comments

Comments

@johnraff
Copy link
Member

johnraff commented Jul 6, 2015

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
Copy link
Contributor

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
Copy link

ghost commented Jul 6, 2015

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

@johnraff
Copy link
Member Author

johnraff commented Jul 7, 2015

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
Copy link
Member Author

johnraff commented Jul 9, 2015

@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
Copy link
Member Author

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

@johnraff johnraff closed this as completed Aug 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants