deragon / autopoweroff

Manage automatic poweroff and other actions (suspend / custom) when specific conditions are reunited.
GNU General Public License v2.0
52 stars 7 forks source link

Bug in SysV Init Script - Does NOT kill autopoweroffd #19

Open RobK88 opened 3 years ago

RobK88 commented 3 years ago

There is a bug in the SysV Init Script in autopoweroffd (all versions). When the autopoweroff-gui restarts autopoweroffd, the daemon is never killed which results in a second (or third etc) autopoweroffd daemon being launched. Multiple autopoweroff daemons running at the same time cause unpredictable and undesirable behaviour, at least on my PC.

This same undesirable behaviour occurs when a user tries to kill autopoweroffd manually (e.g. "sudo service autopoweroff stop") since the killproc function in this init script is also called.

The problem lies with the use of the killproc function used in autopoweroffd's SysV Init script.

First, the autopoweroffd init script uses the wrong sytnax for the killproc function:

killproc -p /var/run/autopoweroff/autopoweroff.pid autopoweroffd

The above syntax is incorrect. One must specify the FULL path name to the daemon not just its name. It should be:

killproc -p /var/run/autopoweroff/autopoweroff.pid /usr/sbin/autopoweroffd

Please see: https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptfunc.html and https://unix.stackexchange.com/questions/110959/how-does-killproc-knows-what-pid-to-kill

Secondly, even if the right syntax is used, the revised init script is unlikely to properly kill autopoweroffd.

When trying to kill autopoweroffd manually from the command line using "sudo kill (Process ID number)", one needs to issue the kill command multiple times before autopoweroffd actually dies. autopoweroffd is stubborn, at least on my PC.

In fact, I was never ever able to get killproc to kill the autopoweroffd even after calling killproc multiple times in a loop. The following code NEVER worked:

#!/bin/bash

# Autopoweroff init script.
#
# For further information on guides on how to write init scripts, see:
#
#   http://wiki.debian.org/LSBInitScripts
#   http://www.debian.org/doc/debian-policy/ch-opersys.html

### BEGIN INIT INFO
# Provides:          autopoweroff
# Required-Start:    $local_fs $syslog
# Required-Stop:     $local_fs $syslog
# Default-Start:     2 3 4 5
# Default-Stop:      0 1 6
# Short-Description: Start daemon at boot time
# Description:       Autopoweroff init script
### END INIT INFO

DESC="Autopoweroff"
NAME=autopoweroff
DAEMON="/usr/sbin/${NAME}d"
PIDFILE="/var/run/$NAME/$NAME.pid"
logfile="/var/log/$NAME.log"

test -f /lib/lsb/init-functions || exit 1
. /lib/lsb/init-functions

start()
{
  log_begin_msg "Starting ${DESC}..."
  start_daemon -p "$PIDFILE" "$DAEMON" >>"${logfile}" 2>&1
  log_end_msg $?
}

stop()
{
  log_begin_msg "Stopping ${DESC}..."
  killproc -p "$PIDFILE" "$DAEMON"
  n=0
  while ( [ "$n" -lt 20 ] && [ -e "$PIDFILE" ] ) ; do
        killproc -p "$PIDFILE" "$DAEMON" -KILL
        n=$(( n + 1 ))
        sleep 2
  done
  if [ -e "$PIDFILE" ] ; then
        log_end_msg 1
  else
        log_end_msg $?
  fi        
case "$1" in
  start)          start;;
  stop)           stop;;
  status)
    status_of_proc -p "$PIDFILE" "$DAEMON" "$NAME"
    ;;
  restart)        stop; start;;
  reload)         stop; start;;
  force-reload)   stop; start;;
  *)              cat <<EOM
Usage: $0 {start|stop|restart|reload|force-reload}
Note:  reload and force-reload actually call restart.
EOM
;;
esac

I was only able to get my updated init script to work when I ditched the use of the killproc function and used the "start-stop-daemon" function instead (with the --retry option).

Here is the script that works on my PC running Anti-X 19 Linux (based on Debian Buster Linux). Place this script in the directory where your distro places init scripts (typically /etc/init.d).

#!/bin/bash

# Autopoweroff init script.
#
# For further information on guides on how to write init scripts, see:
#
#   http://wiki.debian.org/LSBInitScripts
#   http://www.debian.org/doc/debian-policy/ch-opersys.html

### BEGIN INIT INFO
# Provides:          autopoweroff
# Required-Start:    $local_fs $syslog
# Required-Stop:     $local_fs $syslog
# Default-Start:     2 3 4 5
# Default-Stop:      0 1 6
# Short-Description: Start daemon at boot time
# Description:       Autopoweroff init script
### END INIT INFO

DESC="Autopoweroff"
NAME=autopoweroff
USER=root
DAEMON="/usr/sbin/${NAME}d"
PIDFILE="/var/run/$NAME/$NAME.pid"
logfile="/var/log/$NAME.log"

# Exit if the package is not installed
[ -x "$DAEMON" ] || exit 0

# Define LSB log_* functions.
# Depend on lsb-base (>= 3.2-14) to ensure that this file is present
# and status_of_proc is working.

test -f /lib/lsb/init-functions || exit 1
. /lib/lsb/init-functions

# Ensure /run/autopoweroff is there
if [ ! -d "$(dirname "$PIDFILE")" ]; then
    mkdir "$(dirname "$PIDFILE")"
    chown "$USER:$USER" "$(dirname "$PIDFILE")"
    chmod 755 "$(dirname "$PIDFILE")"
fi

#
# Function that starts the daemon/service
#
do_start()
{
    log_daemon_msg "Starting ${DESC}..."
    # Return
    #   0 if daemon has been started
    #   1 if daemon was already running
    #   2 if daemon could not be started
    [ -e "$PIDFILE" ] && PID=$(cat "$PIDFILE")
    if ( [ -e "$PIDFILE" ] && ps -p $PID 1>&2 > /dev/null )
    then
        log_action_end_msg 1 "already running, PID's $PID"
        exit 0 
    elif ( [ -w "$PIDFILE" ] )
    then
        log_warning_msg "PID file found while ${NAME} is not running, removing file."
        rm "$PIDFILE"
    fi

    # start_daemon -p "$PIDFILE" "$DAEMON" >>"${logfile}" 2>&1 || return 2
    # log_end_msg $?
    start-stop-daemon --start --chuid "$USER:$USER" --pidfile="$PIDFILE" --quiet -b --exec "$DAEMON" -- "$DAEMON_ARGS" >>"${logfile}" 2>&1 || return 2
    return 0
}

do_stop()
{
  log_begin_msg "Stopping ${DESC}..."
  if [ ! -w "$PIDFILE" ]
    then
        log_warning_msg "Unable to stop ${DESC}. PID file not found."
        return 4
  fi
  # kill_proc -p "$PIDFILE" "${NAME}d"
  # log_end_msg $?
  start-stop-daemon --stop --oknodo --pidfile="$PIDFILE" --retry=3 || return 1
  return 0
}

case "$1" in
  start)
    do_start
    case "$?" in
        0|1) log_end_msg 0 ;;
        2) log_end_msg 1 ;;
    esac
    ;;
  stop)
    do_stop
    case "$?" in
        0) log_end_msg 0 ;;
        1) log_end_msg 1 ;;
    esac
    ;;
  status)
    status_of_proc -p "$PIDFILE" "$DAEMON" "$NAME"
    ;;
  restart|reload|force-reload)
    do_stop
    case "$?" in
      0)
        log_end_msg 0
        do_start
        case "$?" in
            0) log_end_msg 0 ;;
            *) log_end_msg 1 ;; # Failed to start
        esac
        ;;
      *)
        # Failed to stop
        log_end_msg 1
        ;;
    esac
    ;;
 *)              cat <<EOM
Usage: $0 {start|stop|restart|reload|force-reload}
Note:  reload and force-reload actually call restart.
EOM
;;
esac

As you can see I added some extra error checking code. I also added a "status" function so a user can easily check the status of the autopoweroffd using the "service command" (e.g. "service autopoweroff status" or "service --status-all")

It sure looks like using "start-stop-daemon" over "killproc" is the way to go. But the downside is "start-stop daemon" is not as portable as "killproc". Some older Linux distros may not support the use of "start-stop-daemon".

RobK88 commented 3 years ago

If you do not want to cut and paste, attached is my updated init script for autopoweroffd.

autopoweroff.zip

RobK88 commented 3 years ago

I forgot to mention that if one is running autopoweroffd on an old slow PC, one may need to increase the value in the --retry option in my updated init script. "--retry 2" is probably all one needs. I used "--retry 3" to be safe. Higher retry numbers will increase the time before autopoweroffd stops or restarts.

RobK88 commented 3 years ago

Oh yes, for anyone reading this post, please note the following:

After updating or revising the autopoweroffd init script in /etc/init.d, make sure you run the following commands:

sudo update-rc.d autopoweroff remove
sudo update-rc.d autopoweroff defaults
deragon commented 3 years ago

Yes, thanks Rob. I acknowledge the problem. In fact, there is a problem with autopoweroffd itself that would not shutdown properly. The fix is in Git, but was made after the 4.0.0 release.

See the commit: https://github.com/deragon/autopoweroff/commit/4c644ea469c0b49d918fd0f1c5e86922ba332475

I do not remember how it performed with the SysV script afterwards; I will have to rerun tests again. I also need to check your code too. Thanks for the submission. Without looking at yours, I bet you fixed a few issues.

I am starting a new job and therefore I have much less time these days. But when I will have a moment, I will look into it. In the meanwhile, use the latest code instead of the release if you have not done this.

RobK88 commented 3 years ago

When I get a chance I will check out the git code. But I suspect the init script in git will still not kill autopoweroffd given that the wrong syntax was used for the killproc() function.

You may want to consider ditching killproc() anyway and use start-stop-daemon() instead. stop-stop-daemon() appears to be much more reliable. If you look at the other init scripts for other services already on your Linux PC, you will see that many, if not most, are now using start-stop-daemon().

I addition, please consider adding "status" to the init script as I proposed in my revised script. It is easy to add and will allow users to quickly check if the autopoweroffd is running. (e.g. "service autopoweroff status" or "service --status-all")

deragon commented 2 years ago

More than a year later, I still have this problem. After many other more important fixes, I will tackle this one next. Sorry for the soo long delay... My life is just to busy.

RobK88 commented 2 years ago

I understand. Life can get really hectic. I am glad that you have not forgotten and will tackle this issue next.

Rob


From: Hans Deragon @.> Sent: September 25, 2022 5:53 PM To: deragon/autopoweroff @.> Cc: RobK88 @.>; Author @.> Subject: Re: [deragon/autopoweroff] Bug in SysV Init Script - Does NOT kill autopoweroffd (#19)

More than a year later, I still have this problem. After many other more important fixes, I will tackle this one next. Sorry for the soo long delay... My life is just to busy.

— Reply to this email directly, view it on GitHubhttps://github.com/deragon/autopoweroff/issues/19#issuecomment-1257287133, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AD5RB5KIKAX6KAPIYLHTV4LWADCUFANCNFSM4Z47PNNQ. You are receiving this because you authored the thread.Message ID: @.***>

deragon commented 2 years ago

I did not find direct evidence, but from what I read, Sys-V init.d script are not handled properly by systemd and thus this bug. :(

I am in the process of replacing the Sys-V init.d scripts with systemd. Testing is ongoing. Sys-V init.d files will still be available from the tar file, but the .deb and .rpm packages I am generating are meant for modern distributions running systemd.

And.. found some other bugs which are bizarre and slowing the release of systemd support. But stay tuned.

RobK88 commented 2 years ago

I see from an earlier post that you may have fixed the bug in git for Linux systems running init.d scripts. I was using the stable release version which has the bug.


From: Robert Kennedy @.> Sent: September 30, 2022 1:03 PM To: deragon/autopoweroff @.>; deragon/autopoweroff @.> Cc: Author @.> Subject: Re: [deragon/autopoweroff] Bug in SysV Init Script - Does NOT kill autopoweroffd (#19)

Well I am running autopoweroff on AntiX Linux which does NOT use systemd. It still uses the simpler init.d.

So the bug also exists when you use the init.d scripts.


From: Hans Deragon @.> Sent: September 30, 2022 12:56 PM To: deragon/autopoweroff @.> Cc: RobK88 @.>; Author @.> Subject: Re: [deragon/autopoweroff] Bug in SysV Init Script - Does NOT kill autopoweroffd (#19)

I did not find direct evidence, but from what I read, Sys-V init.d script are not handled properly by systemd and thus this bug. :(

I am in the process of replacing the Sys-V init.d scripts with systemd. Testing is ongoing. Sys-V init.d files will still be available from the tar file, but the .deb and .rpm packages I am generating are meant for modern distributions running systemd.

— Reply to this email directly, view it on GitHubhttps://github.com/deragon/autopoweroff/issues/19#issuecomment-1263805155, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AD5RB5I4FAEV4VEDXREEEZ3WA4LUPANCNFSM4Z47PNNQ. You are receiving this because you authored the thread.Message ID: @.***>

deragon commented 2 years ago

I fixed the problem with release 4.2.0. Please provide feedback.

it-jonas commented 2 years ago

I rolled out the new release on various machines and so far nothing to complain about. Thanks a lot for your afford!

RobK88 commented 2 years ago

Well I am running autopoweroff on AntiX Linux which does NOT use systemd. It still uses the simpler init.d.

So the bug also exists when you use the init.d scripts.


From: Hans Deragon @.> Sent: September 30, 2022 12:56 PM To: deragon/autopoweroff @.> Cc: RobK88 @.>; Author @.> Subject: Re: [deragon/autopoweroff] Bug in SysV Init Script - Does NOT kill autopoweroffd (#19)

I did not find direct evidence, but from what I read, Sys-V init.d script are not handled properly by systemd and thus this bug. :(

I am in the process of replacing the Sys-V init.d scripts with systemd. Testing is ongoing. Sys-V init.d files will still be available from the tar file, but the .deb and .rpm packages I am generating are meant for modern distributions running systemd.

— Reply to this email directly, view it on GitHubhttps://github.com/deragon/autopoweroff/issues/19#issuecomment-1263805155, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AD5RB5I4FAEV4VEDXREEEZ3WA4LUPANCNFSM4Z47PNNQ. You are receiving this because you authored the thread.Message ID: @.***>

RobK88 commented 2 years ago

I would have responded earlier but I have been away. Many thanks for fixing the bug! Looks good. If I see any issues, I will let you know.


From: Jonas @.> Sent: October 4, 2022 6:13 PM To: deragon/autopoweroff @.> Cc: RobK88 @.>; Author @.> Subject: Re: [deragon/autopoweroff] Bug in SysV Init Script - Does NOT kill autopoweroffd (#19)

I rolled out the new release on various machines and so far nothing to complain about. Thanks a lot for your afford!

— Reply to this email directly, view it on GitHubhttps://github.com/deragon/autopoweroff/issues/19#issuecomment-1267637138, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AD5RB5O6HMVTWHP6XDTD2XTWBSTXNANCNFSM4Z47PNNQ. You are receiving this because you authored the thread.Message ID: @.***>