[shepherd] branch master updated: service: Prevent respawn while stopping a service.

[Ludovic Courtès]

This is an automated email from the git hooks/post-receive script.

civodul pushed a commit to branch master
in repository shepherd.

The following commit(s) were added to refs/heads/master by this push:
     new ed6e797  service: Prevent respawn while stopping a service.
ed6e797 is described below

commit ed6e797de773ba053cf580eda9ec889e8e21f9e9
Author: Ludovic Courtès <ludo@gnu.org>
AuthorDate: Mon Nov 14 16:06:01 2022 +0100

    service: Prevent respawn while stopping a service.
    
    This is a followup to 5b08dcab1275e4b88fa0fba00aafb980c6ab4af7.
    
    Contrary what to the comment says in that commit, it is possible for
    SIGCHLD to be received and processed while the 'stop' method is running,
    causing the service being stopped to be respawned.  To prevent this,
    clear the 'running' slot before calling the service's 'stop' method so
    that the process monitor doesn't end up respawning the thing.
    
    * modules/shepherd/service.scm (stop): Set the 'running' slot to #f
    before calling SERVICE's 'stop' method.  Remove inaccurate comment.
    * tests/system-star.sh: Test that.
---
 modules/shepherd/service.scm | 15 ++++++---------
 tests/system-star.sh         | 26 +++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index afb41bf..9e31571 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -456,9 +456,6 @@ NEW-SERVICE."
 canonical names for all of the services which have been stopped (including
 transitive dependent services).  This method will print a warning if SERVICE
 is not already running, and will return SERVICE's canonical name in a list."
-  ;; Note: SIGCHLD resulting from calling SERVICE's 'stop' method won't be
-  ;; handled by the time we're done (in which case we'd end up respawning the
-  ;; service we're trying to stop), unless we explicitly yield.
   (if (not (running? service))
       (begin
         (local-output (l10n "Service ~a is not running.")
@@ -480,9 +477,12 @@ is not already running, and will return SERVICE's 
canonical name in a list."
             ;; Stop the service itself.
             (catch #t
               (lambda ()
-                (apply (slot-ref service 'stop)
-                       (service-running-value service)
-                       args))
+                (let ((running (service-running-value service)))
+                  ;; Mark SERVICE as already stopped to prevent the respawn
+                  ;; machinery from firing upon SIGCHLD.
+                  (slot-set! service 'running #f)
+
+                  (apply (slot-ref service 'stop) running args)))
               (lambda (key . args)
                 ;; Special case: 'root' may quit.
                 (and (eq? root-service service)
@@ -490,9 +490,6 @@ is not already running, and will return SERVICE's canonical 
name in a list."
                      (apply quit args))
                 (caught-error key args)))
 
-            ;; SERVICE is no longer running.
-            (slot-set! service 'running #f)
-
             ;; Reset the list of respawns.
             (slot-set! service 'last-respawns '())
 
diff --git a/tests/system-star.sh b/tests/system-star.sh
index 0af3c64..895525e 100755
--- a/tests/system-star.sh
+++ b/tests/system-star.sh
@@ -43,7 +43,16 @@ cat > "$conf" <<EOF
    #:stop  (lambda _
              (system* "$SHELL" "-c" "echo STOPPING")
              (delete-file "$stamp"))
-   #:respawn? #f))
+   #:respawn? #f)
+ (make <service>
+   #:provides '(test-with-respawn)
+   #:start (make-forkexec-constructor
+             (list "$SHELL" "-cex"
+                   "[ ! -f $PWD/$stamp ] ; touch $PWD/$stamp ; sleep 60"))
+   #:stop  (lambda (pid)
+             (delete-file "$stamp")
+             (zero? (system* "$(type -P kill)" (number->string pid))))
+   #:respawn? #t))
 EOF
 
 rm -f "$pid"
@@ -84,4 +93,19 @@ $herd stop test
 ! test -f "$stamp"
 grep "STOPPING" "$log"
 
+# What about a service with a custom 'stop' procedure that uses 'system*'?
+# Stopping the service should not trigger the respawn machinery.
+$herd start test-with-respawn
+$herd status test-with-respawn | grep "started"
+$herd stop test-with-respawn
+$herd status test-with-respawn | grep "stopped"
+
+for i in `seq 1 5`
+do
+    $herd restart test-with-respawn
+    $herd status test-with-respawn | grep "started"
+done
+$herd stop test-with-respawn
+$herd status test-with-respawn | grep "stopped"
+
 $herd stop root