Subject: bug#54215: closed (Re: bug#54215: [PATCH Shepherd] service: Add #:rlimits parameter to 'exec-command' & co.)

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 21 Mar 2022 12:49:02 +0000

Your bug report

#54215: [PATCH Shepherd] service: Add #:rlimits parameter to 'exec-command' & co.

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
54215: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=54215
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#54215: [PATCH Shepherd] service: Add #:rlimits parameter to 'exec-command' & co.
From: Ludovic Courtès <[email-masked]>
To: Attila Lendvai <[email-masked]>
Cc: [email-masked], Maxime Devos <[email-masked]>
Date: Mon, 21 Mar 2022 13:48:44 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi Attila,

Attila Lendvai <[email-masked]> skribis:

> * modules/shepherd/service.scm (exec-command, fork+exec-command,
> make-forkexec-constructor): Add #:rlimits and honor it.  Reorder keyword args
> where needed to be uniform.

Pushed, at last!

  https://git.savannah.gnu.org/cgit/shepherd.git/commit/?id=3ee9a7193d73821d6f1dd76a745ed5e4bb1a78c8

I took the liberty to change #:rlimits to #:resource-limits, to be
consistent with the naming style used for other keyword arguments.

I also updated ‘doc/shepherd.texi’ and made sure the commit log mentions
all the changes.

Thank you for this welcome addition, and apologies for the delay!

Ludo’.

Subject: [PATCH Shepherd] service: Add #:rlimits parameter to 'exec-command' & co.
From: Attila Lendvai <[email-masked]>
To: [email-masked]
Cc: Attila Lendvai <[email-masked]>
Date: Tue, 1 Mar 2022 19:12:43 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

* modules/shepherd/service.scm (exec-command, fork+exec-command,
make-forkexec-constructor): Add #:rlimits and honor it.  Reorder keyword args
where needed to be uniform.
---

this patch supersedes my previous CALL-IN-FORK proposal:

https://issues.guix.gnu.org/54205

i will either close that, or maybe do the internal refactor. we'll see.

 modules/shepherd/service.scm | 26 ++++++++++++++++++--------
 tests/forking-service.sh     | 15 +++++++++++++--
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index ad8608b..c6f0f4e 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -787,7 +787,8 @@ daemon writing FILE is running in a separate PID namespace."
                        (directory (default-service-directory))
                        (file-creation-mask #f)
                        (create-session? #t)
-                       (environment-variables (default-environment-variables)))
+                       (environment-variables (default-environment-variables))
+                       (rlimits '()))
   "Run COMMAND as the current process from DIRECTORY, with FILE-CREATION-MASK
 if it's true, and with ENVIRONMENT-VARIABLES (a list of strings like
 \"PATH=/bin\").  File descriptors 1 and 2 are kept as is or redirected to
@@ -795,6 +796,9 @@ LOG-FILE if it's true, whereas file descriptor 0 (standard input) points to
 /dev/null; all other file descriptors are closed prior to yielding control to
 COMMAND.  When CREATE-SESSION? is true, call 'setsid' first.
 
+Guile's SETRLIMIT function will be applied on the entries in RLIMITS.  For
+example a valid value would be '((nproc 10 100) (nofile 4096 4096)).
+
 By default, COMMAND is run as the current user.  If the USER keyword
 argument is present and not false, change to USER immediately before
 invoking COMMAND.  USER may be a string, indicating a user name, or a
@@ -808,6 +812,8 @@ false."
        ;; Programs such as 'mingetty' expect this.
        (setsid))
 
+     (for-each (cut apply setrlimit <>) rlimits)
+
      (chdir directory)
      (environ environment-variables)
 
@@ -893,7 +899,8 @@ false."
                             (file-creation-mask #f)
                             (create-session? #t)
                             (environment-variables
-                             (default-environment-variables)))
+                             (default-environment-variables))
+                            (rlimits '()))
   "Spawn a process that executed COMMAND as per 'exec-command', and return
 its PID."
   ;; Install the SIGCHLD handler if this is the first fork+exec-command call.
@@ -924,7 +931,8 @@ its PID."
                           #:directory directory
                           #:file-creation-mask file-creation-mask
                           #:create-session? create-session?
-                          #:environment-variables environment-variables))
+                          #:environment-variables environment-variables
+                          #:rlimits rlimits))
           pid))))
 
 (define* (make-forkexec-constructor command
@@ -932,15 +940,16 @@ its PID."
                                     (user #f)
                                     (group #f)
                                     (supplementary-groups '())
+                                    (log-file #f)
                                     (directory (default-service-directory))
-                                    (environment-variables
-                                     (default-environment-variables))
                                     (file-creation-mask #f)
                                     (create-session? #t)
+                                    (environment-variables
+                                     (default-environment-variables))
+                                    (rlimits '())
                                     (pid-file #f)
                                     (pid-file-timeout
-                                     (default-pid-file-timeout))
-                                    (log-file #f))
+                                     (default-pid-file-timeout)))
   "Return a procedure that forks a child process, closes all file
 descriptors except the standard output and standard error descriptors, sets
 the current directory to @var{directory}, sets the umask to
@@ -978,7 +987,8 @@ start."
                                   #:file-creation-mask file-creation-mask
                                   #:create-session? create-session?
                                   #:environment-variables
-                                  environment-variables)))
+                                  environment-variables
+                                  #:rlimits rlimits)))
       (if pid-file
           (match (read-pid-file pid-file
                                 #:max-delay pid-file-timeout
diff --git a/tests/forking-service.sh b/tests/forking-service.sh
index bd9aac9..a745bf4 100644
--- a/tests/forking-service.sh
+++ b/tests/forking-service.sh
@@ -25,6 +25,7 @@ conf="t-conf-$$"
 log="t-log-$$"
 pid="t-pid-$$"
 service_pid="t-service-pid-$$"
+service_nofiles="t-service-nofiles-$$"
 service2_pid="t-service2-pid-$$"
 service2_started="t-service2-starts-$$"
 
@@ -49,14 +50,15 @@ cat > "$conf"<<EOF
 (default-pid-file-timeout 10)
 
 (define %command
-  '("$SHELL" "-c" "sleep 600 & echo \$! > $PWD/$service_pid"))
+  '("$SHELL" "-c" "ulimit -n >$PWD/$service_nofiles; sleep 600 & echo \$! > $PWD/$service_pid"))
 
 (register-services
  (make <service>
    ;; A service that forks into a different process.
    #:provides '(test)
    #:start (make-forkexec-constructor %command
-                                      #:pid-file "$PWD/$service_pid")
+                                      #:pid-file "$PWD/$service_pid"
+                                      #:rlimits '((nofile 1567 1567)))
    #:stop  (make-kill-destructor)
    #:respawn? #f))
 
@@ -125,6 +127,15 @@ $herd status test2 | grep started
 test "`cat $PWD/$service2_started`" = "started
 started"
 
+
+
+# test if nofiles was set properly
+test -f "$service_nofiles"
+nofiles_value="`cat $service_nofiles`"
+test 1567 -eq $nofiles_value
+
+
+
 # Try to trigger eventual race conditions, when killing a process between fork
 # and execv calls.
 for i in `seq 1 50`
-- 
2.34.0