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.

Sign up for GitHub

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

Jump to bottom

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

Open
RobK88 opened this issue Mar 27, 2021 · 13 comments
Open

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

RobK88 opened this issue Mar 27, 2021 · 13 comments
Assignees
@deragon
Labels
bug

Comments

@RobK88
Copy link

RobK88 commented Mar 27, 2021 •
edited
Loading

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

RobK88 commented Mar 27, 2021 •
edited
Loading

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

autopoweroff.zip

@RobK88
Copy link
Author

RobK88 commented Mar 27, 2021 •
edited
Loading

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

RobK88 commented Mar 27, 2021 •
edited
Loading

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 deragon self-assigned this Mar 29, 2021
@deragon deragon added the bug label Mar 29, 2021
@deragon
Copy link
Owner

deragon commented Mar 29, 2021

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: 4c644ea

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

RobK88 commented Mar 31, 2021 •
edited
Loading

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

deragon commented Sep 25, 2022

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

RobK88 commented Sep 26, 2022 via email

@deragon deragon mentioned this issue Sep 30, 2022
Closed
@deragon
Copy link
Owner

deragon commented Sep 30, 2022 •
edited
Loading

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

RobK88 commented Sep 30, 2022 via email

@deragon
Copy link
Owner

deragon commented Oct 1, 2022

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

@it-jonas
Copy link

it-jonas commented Oct 4, 2022

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

@RobK88
Copy link
Author

RobK88 commented Oct 11, 2022 via email

@RobK88
Copy link
Author

RobK88 commented Oct 15, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@deragon deragon

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@deragon @RobK88 @it-jonas