n0bel / PiClock

A Fancy Clock built around a monitor and a Raspberry Pi
MIT License
566 stars 182 forks source link

switcher does not always kill running PiClock process #132

Closed BertLindeman closed 5 years ago

BertLindeman commented 5 years ago

Tried to implement a day/night configuration so the clock has more bedside possibility as indicated by #14

Using crontab to issue /home/pi/PiClock/switcher.sh Config-bert-d and /home/pi/PiClock/switcher.sh Config-bert-n. The net result was that more and more PiClocks were running.

So I tried the pkill command by hand. I found it hard to get a regex working on pkill until I set the regex in a variable. And the -HUP signal did not do the trick.

My startup and switcher both use another path to send the logging to, so the log goes to tempfs and not to the sd-card. Therefor I show only the pkill part of my switcher (added at the top):

# /BL 20190201 pkill with "-f" does not kill the clock if any config has been specified on startup

cd $HOME/PiClock
# pkill -HUP -f PyQtPiClock.py
piclockregex="python -u PyQtPiClock.py*"
pkill -f "$piclockregex"
# show python tasks still running (e.g. the TempServer):
pgrep -a python

A pull request can be made if you want me to do so.

n0bel commented 5 years ago

@BertLindeman I've just finished experimenting with this every way I can think of and I can't get it to fail. switcher.sh works fine just as is...
What OS or pkill version are you using?

~/PiClock $ pkill -V
pkill from procps-ng 3.3.9

That said, pkill properly in this manner for in all the Raspbian OS's I've ever used.

Also regarding your regex... Without quotes it would be looking for a file match first (shell expansion). Also you're using shell pattern matching syntax, not regex. In a regex, '*' means any number of matches on the prior character.. So it would be looking for .py or .pyy or .pyyy... etc. The proper regex usage (single quoted to escape shell escaping and expansion) would be

pkill -f 'python -u PyQtPiClock\.py.*'

But still all that said.. It will not fail for me.

Perhaps it is a permissions issue, or it has more to do with -HUP than -f? In my setups PiClock is started by desktop autostart, so therefore the user is "pi". My switching is done by the user "pi"'s crontab.

I do appreciate all the help!

Kevin

BertLindeman commented 5 years ago

Thanks Kevin for looking into this.

I am running:

Raspbian GNU/Linux 9.6 (stretch) GNU/Linux
Kernel: 4.14.79-v7+ armv7l

pkill -V shows: pkill from procps-ng 3.3.12

The doc for "-f" on my rpi2:

-f, --full
              The pattern is normally only matched against the process name.  When -f is set, the full command line is used.

I interpret this as: if the clock has been started using a named config, that process would not match: pkill -HUP -f PyQtPiClock.py This may be a wrong assumption, or plainly wrong in the doc.

In my opinion python -u PyQtPiClock.py Config-bert-d should not match this pkill command. But if as it does . . . and always matched as you used it: I will not complain.

Re. regex: I know that the regex needs to be quoted to prevent shell expanding. Running user for the clock is pi and the switching is also in pi crontab.

Have reverted my update on switcher.sh and will take a look at the switches in my user pi crontab. Will look at what happens and report back. I do not understand what/why pkill and pgrep issued warnings this morning when I tried to use a regex... ==-->> later

n0bel commented 5 years ago

I interpret this as: if the clock has been started using a named config, that process would not match:

Yes.. however the "pattern" is a regex.

OPERANDS
       pattern
              Specifies an Extended Regular Expression for matching against the process names or command lines.

Regex "match" without ^ and $ will match anywhere in the string.

BertLindeman commented 5 years ago

Ah, THAT was missing in my mind. Thank you for thinking with me. Confirming:

pgrep -af PyQt
973 python -u PyQtPiClock.py

But something is not 100% OK somewhere. User pi has crontab entries to test with:

10 19 * * *     /home/pi/PiClock/switcher.sh Config-bert-n
20 19 * * *     /home/pi/PiClock/switcher.sh Config-bert-d
00 20 * * *     /home/pi/PiClock/switcher.sh Config-bert-n
10 20 * * *     /home/pi/PiClock/switcher.sh Config-bert-d

The clock running before the crontab entries matched was started at boot without a named config. That running process is process# 973 (For later reference)

The current switcher starts with the original pkill:

 less /home/pi/PiClock/switcher.sh
cd $HOME/PiClock
pkill -HUP -f PyQtPiClock.py
cd Clock

After two hits in this crontab Config-bert-d is active and the -n one has vanished (OK). As ps shows:

pi         963  0.1  1.0  43392 10316 ?        Sl   12:34   0:28 python TempServer.py
pi         973 11.0  8.2 176548 78244 ?        Sl   12:34  45:29 python -u PyQtPiClock.py
pi        5340 11.6  6.9 164616 65660 ?        Sl   19:20   0:37 python -u PyQtPiClock.py Config-bert-d
pi        5377  0.0  0.0   5996   588 pts/0    R+   19:25   0:00 grep --color=always -i python

The Config-bert-n clock has been killed, but the boot-started clock still runs. So the concern is not as big as I thought. Not ALL clocks keep running.

So there is some problem stopping the boot-started clock. In my case the clock is started on start of x session of user pi via: /home/pi/.config/autostart/PiClock.desktop

[Desktop Entry]
Encoding=UTF-8
Type=Application
Name=PiClock
Comment=
Exec=/bin/sh -c "/bin/sh ~/PiClock/startup.sh -m 15"
StartupNotify=false
Terminal=false
Hidden=false
Icon=orage_globaltime
Path=~/PiClock
#
# No idea where orage_globaltime should be / come from.
#
# removed this start from pi crontab....
# @reboot sh /home/pi/PiClock/startup.sh

Have to stop for a few moments, so documenting progress sofar.

n0bel commented 5 years ago

try -INT instead of -HUP HUP is 'hang up' which comes from old school modem days where the tty looses DCD, and sends a HUP signal to the shell. There is no tty associated with the process (the ?) so maybe thats it.

-INT will simulate a control-c Then try -TERM which is a normal termination signal And lastly try -KILL which should be a final killer (not a graceful process shutdown)

https://www.quora.com/What-is-the-difference-between-the-SIGINT-and-SIGTERM-signals-in-Linux-What%E2%80%99s-the-difference-between-the-SIGKILL-and-SIGSTOP-signals

n0bel commented 5 years ago

That matches my startup method.

~ $ cat /home/pi/.config/autostart/Pi*                                                                                                           
[Desktop Entry]
Encoding=UTF-8
Type=Application
Name=PiClock
Comment=
Exec=/bin/sh -c "/bin/sh ~/PiClock/startup.sh -m 15"
StartupNotify=false
Terminal=false
Hidden=false
BertLindeman commented 5 years ago

Lovely article you pointed to.

While doing something else keeping an eye on your updates, Updated switcher now with -INT and awaiting next cron switch...

cron hits and YES original clock has gone to the "happy process grounds".

So the change on pkill from -HUP into -INT is the good one for me.

Usualy I do stop a running clock with ctrl-c so -INT sounds quite plausable.

BertLindeman commented 5 years ago

renamed this issue from "switcher does not kill running PiClock process if a config has been specified" into "switcher does not always kill running PiClock process"