[shepherd] 02/02: services: Kill the process that did not create its PID file.

[Ludovic Courtès]

civodul pushed a commit to branch master
in repository shepherd.

commit a8fa86bb23cc457e2cd603ad505c964bc8dc889e
Author: Ludovic Courtès <address@hidden>
Date:   Sat Nov 26 19:27:16 2016 +0100

    services: Kill the process that did not create its PID file.
    
    Reported by address@hidden (宋文武)
    at <https://lists.gnu.org/archive/html/guix-devel/2016-11/msg00947.html>.
    
    * modules/shepherd/service.scm (read-pid-file): Return #f upon ENOENT
    after MAX-DELAY has expired.
    (make-forkexec-constructor): When 'read-pid-file' returns #f,
    do (kill pid SIGTERM).
    * tests/pid-file.sh: New file.
    * Makefile.am (TESTS): Add it.
---
 .gitignore                   |    1 +
 Makefile.am                  |    1 +
 modules/shepherd/service.scm |   29 +++++++++--------
 tests/pid-file.sh            |   70 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/.gitignore b/.gitignore
index fe35476..734c71c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -56,3 +56,4 @@ Makefile
 Makefile.in
 /tests/respawn-throttling.log
 /tests/respawn-throttling.trs
+/tests/pid-file.log
diff --git a/Makefile.am b/Makefile.am
index 9767d77..65c55f7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -184,6 +184,7 @@ TESTS =                                             \
   tests/respawn-throttling.sh                  \
   tests/misbehaved-client.sh                   \
   tests/no-home.sh                             \
+  tests/pid-file.sh                            \
   tests/status-sexp.sh                         \
   tests/sigint.sh
 
diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index 1ef6272..26f29aa 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -667,8 +667,8 @@ set when starting a service."
 
 (define* (read-pid-file file #:key (max-delay 5))
   "Wait for MAX-DELAY seconds for FILE to show up, and read its content as a
-number.  Return #f if FILE does not contain a number; otherwise return the
-number that was read (a PID)."
+number.  Return #f if FILE was not created or does not contain a number;
+otherwise return the number that was read (a PID)."
   (define start (current-time))
   (let loop ()
     (catch 'system-error
@@ -678,14 +678,14 @@ number that was read (a PID)."
           (call-with-input-file file get-string-all))))
       (lambda args
         (let ((errno (system-error-errno args)))
-          (if (and (= ENOENT errno)
-                   (< (current-time) (+ start max-delay)))
-              (begin
-                ;; FILE does not exist yet, so wait and try again.
-                ;; XXX: Ideally we would yield to the main event loop
-                ;; and/or use inotify.
-                (sleep 1)
-                (loop))
+          (if (= ENOENT errno)
+              (and (< (current-time) (+ start max-delay))
+                   (begin
+                     ;; FILE does not exist yet, so wait and try again.
+                     ;; XXX: Ideally we would yield to the main event loop
+                     ;; and/or use inotify.
+                     (sleep 1)
+                     (loop)))
               (apply throw args)))))))
 
 (define* (exec-command command
@@ -856,8 +856,13 @@ start."
                                         #:environment-variables
                                         environment-variables)))
             (if pid-file
-                (read-pid-file pid-file
-                               #:max-delay pid-file-timeout)
+                (match (read-pid-file pid-file
+                                      #:max-delay pid-file-timeout)
+                  (#f
+                   (catch-system-error (kill pid SIGTERM))
+                   #f)
+                  ((? integer? pid)
+                   pid))
                 pid)))))
      ((program . program-args)
       ;; The old form, documented until 0.1 included.
diff --git a/tests/pid-file.sh b/tests/pid-file.sh
new file mode 100644
index 0000000..d1be416
--- /dev/null
+++ b/tests/pid-file.sh
@@ -0,0 +1,70 @@
+# GNU Shepherd --- Test the #:pid-file option of 'make-forkexec-constructor'.
+# Copyright © 2016 Ludovic Courtès <address@hidden>
+#
+# This file is part of the GNU Shepherd.
+#
+# The GNU Shepherd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or (at
+# your option) any later version.
+#
+# The GNU Shepherd is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with the GNU Shepherd.  If not, see <http://www.gnu.org/licenses/>.
+
+shepherd --version
+herd --version
+
+socket="t-socket-$$"
+conf="t-conf-$$"
+log="t-log-$$"
+pid="t-pid-$$"
+service_pid="t-service-pid-$$"
+
+herd="herd -s $socket"
+
+trap "cat $log || true; rm -f $socket $conf $service_pid $log;
+      test -f $pid && kill \`cat $pid\` || true; rm -f $pid" EXIT
+
+cat > "$conf"<<EOF
+(use-modules (ice-9 match))
+
+(define %command
+  '("$SHELL" "-c" "echo \$\$ > $PWD/$service_pid ; sleep 600"))
+
+(register-services
+ (make <service>
+   ;; A service that never produces its PID file, yet leaves a process
+   ;; behind it.
+   #:provides '(test)
+   #:start (make-forkexec-constructor %command
+                                      #:pid-file "/does-not-exist"
+                                      #:pid-file-timeout 2)
+   #:stop  (make-kill-destructor)
+   #:respawn? #f))
+EOF
+
+rm -f "$pid"
+shepherd -I -s "$socket" -c "$conf" -l "$log" --pid="$pid" &
+
+# Wait till it's ready.
+while ! test -f "$pid" ; do sleep 0.3 ; done
+
+shepherd_pid="`cat $pid`"
+
+# The service is expected to fail to start.
+if $herd start test
+then false; else true; fi
+
+# Make sure it is marked as stopped.
+$herd status test | grep stopped
+
+test -f "$service_pid"
+
+# Make sure it did not leave a process behind it.
+if kill -0 `cat "$service_pid"`
+then false; else true; fi