GNU bug report logs

#30950 [PATCH shepherd]: Update required guile version, and remove some hacks

PackageSource(s)Maintainer(s)
guix-patches PTS Buildd Popcon
Reply or subscribe to this bug. View this bug as an mbox, status mbox, or maintainer mbox

Report forwarded to guix-patches@gnu.org:
bug#30950; Package guix-patches. (Mon, 26 Mar 2018 11:56:01 GMT) (full text, mbox, link).


Acknowledgement sent to Carlo Zancanaro <carlo@zancanaro.id.au>:
New bug report received and forwarded. Copy sent to guix-patches@gnu.org. (Mon, 26 Mar 2018 11:56:01 GMT) (full text, mbox, link).


Message #5 received at submit@debbugs.gnu.org (full text, mbox, reply):

From: Carlo Zancanaro <carlo@zancanaro.id.au>
To: guix-patches@gnu.org
Subject: [PATCH shepherd]: Update required guile version, and remove some hacks
Date: Mon, 26 Mar 2018 22:55:03 +1100
[Message part 1 (text/plain, inline)]
These patches bring the Shepherd's Guile dependencies in line with 
Guix, and remove some hacks that were required for old Guile 
problems.

I'm not very familiar with autotools, but I think I got the 
configure incantation right (I stole it from Guix).

[0001-Update-Guile-dependency-to-2.0.13-or-later.patch (text/x-patch, inline)]
From 8c812534137a5dc17dd8073706983c451d26f2db Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Mon, 26 Mar 2018 14:44:18 +1100
Subject: [PATCH 1/3] Update Guile dependency to 2.0.13 or later

* README (Requirements): Change 2.x to 2.0.13 or later.
* configure.ac: Check for 2.0.13 or later if Guile 2.0 is detected.
---
 README       | 7 ++++---
 configure.ac | 4 ++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/README b/README
index 88613aa..1237e2c 100644
--- a/README
+++ b/README
@@ -16,9 +16,10 @@ daemon-managing daemon.
 ** Requirements
 
 This program requires Guile (the GNU Ubiquitous Intelligent Language
-for Extension), version 2.x.  It uses GOOPS, but as GOOPS is part of
-Guile, a normal Guile installation is sufficient.  It also uses
-readline, though it does not really depend on it.
+for Extension), version 2.0.13 or later (including 2.2.x).  It uses
+GOOPS, but as GOOPS is part of Guile, a normal Guile installation is
+sufficient.  It also uses readline, though it does not really depend
+on it.
 
 GNU Make is required to build the Shepherd.
 
diff --git a/configure.ac b/configure.ac
index d768885..9d8c2aa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,6 +27,10 @@ GUILE_PKG([2.2 2.0])
 dnl Checks for programs.
 GUILE_PROGS
 
+if test "x$GUILE_EFFECTIVE_VERSION" = "x2.0"; then
+  PKG_CHECK_MODULES([GUILE], [guile-2.0 >= 2.0.13])
+fi
+
 guilemoduledir="${datarootdir}/guile/site/$GUILE_EFFECTIVE_VERSION"
 guileobjectdir="${libdir}/guile/$GUILE_EFFECTIVE_VERSION/site-ccache"
 AC_SUBST([guilemoduledir])
-- 
2.16.2

[0002-Remove-EINTR-safe-and-all-references-to-it.patch (text/x-patch, inline)]
From e11708aba0fbafd4c83273ee1fa5147e54d1c80e Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Mon, 26 Mar 2018 14:49:18 +1100
Subject: [PATCH 2/3] Remove EINTR-safe, and all references to it.

* modules/shepherd/support.scm (EINTR-safe): Remove procedure and its export.
* modules/shepherd/service.scm (system*, system*): Remove now-unnecessary
  procedures.
  (waitpid*): Remove references to EINTR-safe.
* modules/shepherd.scm (main): Remove references to EINTR-safe.
---
 modules/shepherd.scm         |  7 +------
 modules/shepherd/service.scm | 35 +++++++++++++----------------------
 modules/shepherd/support.scm | 14 --------------
 3 files changed, 14 insertions(+), 42 deletions(-)

diff --git a/modules/shepherd.scm b/modules/shepherd.scm
index fede338..5d97598 100644
--- a/modules/shepherd.scm
+++ b/modules/shepherd.scm
@@ -241,12 +241,7 @@
           ;; Get commands from the standard input port.
           (process-textual-commands (current-input-port))
           ;; Process the data arriving at a socket.
-          (let ((sock   (open-server-socket socket-file))
-
-                ;; With Guile <= 2.0.9, we can get a system-error exception for
-                ;; EINTR, which happens anytime we receive a signal, such as
-                ;; SIGCHLD.  Thus, wrap the 'accept' call.
-                (accept (EINTR-safe accept)))
+          (let ((sock   (open-server-socket socket-file)))
 
             ;; Possibly write out our PID, which means we're ready to accept
             ;; connections.  XXX: What if we daemonized already?
diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index 7b062a1..93d3779 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -590,13 +590,6 @@ results."
                (apply action service the-action args))
              which-services))))
 
-;; EINTR-safe versions of 'system' and 'system*'.
-
-(define system*
-  (EINTR-safe (@ (guile) system*)))
-
-(define system
-  (EINTR-safe (@ (guile) system)))
 
 
 
@@ -981,21 +974,19 @@ returned in unspecified."
   (hashq-ref %services name '()))
 
 (define waitpid*
-  (let ((waitpid (EINTR-safe waitpid)))
-    (lambda (what flags)
-      "Like 'waitpid', but EINTR-safe, and return (0 . _) when there's no
-child left."
-      (catch 'system-error
-        (lambda ()
-          (waitpid what flags))
-        (lambda args
-          ;; Did we get ECHILD or something?  If we did, that's a problem,
-          ;; because this procedure is supposed to be called only upon
-          ;; SIGCHLD.
-          (let ((errno (system-error-errno args)))
-            (local-output "warning: 'waitpid' ~a failed unexpectedly: ~a"
-                          what (strerror errno))
-            '(0 . #f)))))))
+  (lambda (what flags)
+    "Like 'waitpid', and return (0 . _) when there's no child left."
+    (catch 'system-error
+      (lambda ()
+        (waitpid what flags))
+      (lambda args
+        ;; Did we get ECHILD or something?  If we did, that's a problem,
+        ;; because this procedure is supposed to be called only upon
+        ;; SIGCHLD.
+        (let ((errno (system-error-errno args)))
+          (local-output "warning: 'waitpid' ~a failed unexpectedly: ~a"
+                        what (strerror errno))
+          '(0 . #f))))))
 
 (define (handle-SIGCHLD signum)
   "Handle SIGCHLD, possibly by respawning the service that just died, or
diff --git a/modules/shepherd/support.scm b/modules/shepherd/support.scm
index 380866e..9f02719 100644
--- a/modules/shepherd/support.scm
+++ b/modules/shepherd/support.scm
@@ -30,7 +30,6 @@
 
             catch-system-error
             with-system-error-handling
-            EINTR-safe
             with-atomic-file-output
             mkdir-p
             with-directory-excursion
@@ -127,19 +126,6 @@ turned into user error messages."
    (lambda ()
      body ...)))
 
-(define (EINTR-safe proc)
-  "Wrap PROC so that if a 'system-error' exception with EINTR is raised (that
-was possible up to Guile 2.0.9 included) the call to PROC is restarted."
-  (lambda args
-    (let loop ()
-      (catch 'system-error
-        (lambda ()
-          (apply proc args))
-        (lambda args
-          (if (= EINTR (system-error-errno args))
-              (loop)
-              (apply throw args)))))))
-
 (define (with-atomic-file-output file proc)       ;copied from Guix
   "Call PROC with an output port for the file that is going to replace FILE.
 Upon success, FILE is atomically replaced by what has been written to the
-- 
2.16.2

[0003-Remove-SIGALRM-hack.patch (text/x-patch, inline)]
From 63bc9339d88d8f1bd8a9b366774ce8e33d76dd00 Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Mon, 26 Mar 2018 14:55:32 +1100
Subject: [PATCH 3/3] Remove SIGALRM hack.

* modules/shepherd.scm (main): Remove SIGALRM hack for guile <= 2.0.9.
---
 modules/shepherd.scm | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/modules/shepherd.scm b/modules/shepherd.scm
index 5d97598..69fd69d 100644
--- a/modules/shepherd.scm
+++ b/modules/shepherd.scm
@@ -207,15 +207,6 @@
                (apply format #f (gettext (cadr args)) (caddr args))
                (quit 1))))
 
-      (when (provided? 'threads)
-        ;; XXX: This terrible hack allows us to make sure that signal handlers
-        ;; get a chance to run in a timely fashion.  Without it, after an EINTR,
-        ;; we could restart the accept(2) call below before the corresponding
-        ;; async has been queued.  See the thread at
-        ;; <https://lists.gnu.org/archive/html/guile-devel/2013-07/msg00004.html>.
-        (sigaction SIGALRM (lambda _ (alarm 1)))
-        (alarm 1))
-
       ;; Stop everything when we get SIGINT.  When running as PID 1, that means
       ;; rebooting; this is what happens when pressing ctrl-alt-del, see
       ;; ctrlaltdel(8).
-- 
2.16.2

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches@gnu.org:
bug#30950; Package guix-patches. (Thu, 29 Mar 2018 20:15:01 GMT) (full text, mbox, link).


Message #8 received at 30950@debbugs.gnu.org (full text, mbox, reply):

From: ludo@gnu.org (Ludovic Courtès)
To: Carlo Zancanaro <carlo@zancanaro.id.au>
Cc: 30950@debbugs.gnu.org
Subject: Re: [bug#30950] [PATCH shepherd]: Update required guile version, and remove some hacks
Date: Thu, 29 Mar 2018 22:14:01 +0200
Hello!

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> I'm not very familiar with autotools, but I think I got the configure
> incantation right (I stole it from Guix).

Well done.  :-)

> From 8c812534137a5dc17dd8073706983c451d26f2db Mon Sep 17 00:00:00 2001
> From: Carlo Zancanaro <carlo@zancanaro.id.au>
> Date: Mon, 26 Mar 2018 14:44:18 +1100
> Subject: [PATCH 1/3] Update Guile dependency to 2.0.13 or later
>
> * README (Requirements): Change 2.x to 2.0.13 or later.
> * configure.ac: Check for 2.0.13 or later if Guile 2.0 is detected.

LGTM.

> From e11708aba0fbafd4c83273ee1fa5147e54d1c80e Mon Sep 17 00:00:00 2001
> From: Carlo Zancanaro <carlo@zancanaro.id.au>
> Date: Mon, 26 Mar 2018 14:49:18 +1100
> Subject: [PATCH 2/3] Remove EINTR-safe, and all references to it.
>
> * modules/shepherd/support.scm (EINTR-safe): Remove procedure and its export.
> * modules/shepherd/service.scm (system*, system*): Remove now-unnecessary
>   procedures.
>   (waitpid*): Remove references to EINTR-safe.
> * modules/shepherd.scm (main): Remove references to EINTR-safe.

LGTM.

> From 63bc9339d88d8f1bd8a9b366774ce8e33d76dd00 Mon Sep 17 00:00:00 2001
> From: Carlo Zancanaro <carlo@zancanaro.id.au>
> Date: Mon, 26 Mar 2018 14:55:32 +1100
> Subject: [PATCH 3/3] Remove SIGALRM hack.
>
> * modules/shepherd.scm (main): Remove SIGALRM hack for guile <= 2.0.9.
> ---
>  modules/shepherd.scm | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/modules/shepherd.scm b/modules/shepherd.scm
> index 5d97598..69fd69d 100644
> --- a/modules/shepherd.scm
> +++ b/modules/shepherd.scm
> @@ -207,15 +207,6 @@
>                 (apply format #f (gettext (cadr args)) (caddr args))
>                 (quit 1))))
>  
> -      (when (provided? 'threads)
> -        ;; XXX: This terrible hack allows us to make sure that signal handlers
> -        ;; get a chance to run in a timely fashion.  Without it, after an EINTR,
> -        ;; we could restart the accept(2) call below before the corresponding
> -        ;; async has been queued.  See the thread at
> -        ;; <https://lists.gnu.org/archive/html/guile-devel/2013-07/msg00004.html>.
> -        (sigaction SIGALRM (lambda _ (alarm 1)))
> -        (alarm 1))

Unfortunately I think the problem remains.  That’s one of the reasons
for using signalfd(2).

Can you create an account on Savannah so I can add you to the group and
let you push the first two patches?  :-)

Thank you!

Ludo’.




Information forwarded to guix-patches@gnu.org:
bug#30950; Package guix-patches. (Thu, 29 Mar 2018 20:32:01 GMT) (full text, mbox, link).


Message #11 received at 30950@debbugs.gnu.org (full text, mbox, reply):

From: Leo Famulari <leo@famulari.name>
To: Ludovic Courtès <ludo@gnu.org>
Cc: Carlo Zancanaro <carlo@zancanaro.id.au>, 30950@debbugs.gnu.org
Subject: Re: [bug#30950] [PATCH shepherd]: Update required guile version, and remove some hacks
Date: Thu, 29 Mar 2018 16:31:25 -0400
[Message part 1 (text/plain, inline)]
On Thu, Mar 29, 2018 at 10:14:01PM +0200, Ludovic Courtès wrote:
> Can you create an account on Savannah so I can add you to the group and
> let you push the first two patches?  :-)

Welcome! Please make sure to read the HACKING file in our Git repo :)
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches@gnu.org:
bug#30950; Package guix-patches. (Thu, 29 Mar 2018 21:28:01 GMT) (full text, mbox, link).


Message #14 received at 30950@debbugs.gnu.org (full text, mbox, reply):

From: Carlo Zancanaro <carlo@zancanaro.id.au>
To: Ludovic Courtès <ludo@gnu.org>
Cc: 30950@debbugs.gnu.org
Subject: Re: [bug#30950] [PATCH shepherd]: Update required guile version, and remove some hacks
Date: Fri, 30 Mar 2018 08:27:41 +1100
[Message part 1 (text/plain, inline)]
On Thu, Mar 29 2018, Ludovic Courtès wrote:
> Can you create an account on Savannah so I can add you to the 
> group and let you push the first two patches?  :-)

I'm working on this. Something has gone wrong, and now I can't 
activate my account (because apparently I have the password 
wrong), and I can't reset my password (because my account isn't 
activated yet). I'll let you know when I get it sorted out.

Carlo
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches@gnu.org:
bug#30950; Package guix-patches. (Thu, 29 Mar 2018 21:37:02 GMT) (full text, mbox, link).


Message #17 received at 30950@debbugs.gnu.org (full text, mbox, reply):

From: Carlo Zancanaro <carlo@zancanaro.id.au>
To: Leo Famulari <leo@famulari.name>
Cc: Ludovic Courtès <ludo@gnu.org>, 30950@debbugs.gnu.org
Subject: Re: [bug#30950] [PATCH shepherd]: Update required guile version, and remove some hacks
Date: Fri, 30 Mar 2018 08:36:36 +1100
[Message part 1 (text/plain, inline)]
On Thu, Mar 29 2018, Leo Famulari wrote:
> Welcome! Please make sure to read the HACKING file in our Git 
> repo :)

Does the Guix HACKING file apply to the Shepherd, too? In 
particular, it doesn't look like commits on the Shepherd are 
signed?

Carlo
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches@gnu.org:
bug#30950; Package guix-patches. (Fri, 30 Mar 2018 08:10:02 GMT) (full text, mbox, link).


Message #20 received at 30950@debbugs.gnu.org (full text, mbox, reply):

From: ludo@gnu.org (Ludovic Courtès)
To: Carlo Zancanaro <carlo@zancanaro.id.au>
Cc: 30950@debbugs.gnu.org, Leo Famulari <leo@famulari.name>
Subject: Re: [bug#30950] [PATCH shepherd]: Update required guile version, and remove some hacks
Date: Fri, 30 Mar 2018 10:09:32 +0200
Hello,

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> On Thu, Mar 29 2018, Leo Famulari wrote:
>> Welcome! Please make sure to read the HACKING file in our Git repo
>> :)
>
> Does the Guix HACKING file apply to the Shepherd, too? 

Yeah, I think we should follow the same rules, roughly.

> In particular, it doesn't look like commits on the Shepherd are
> signed?

Indeed, now’s the time to fix it!

Ludo’.




Information forwarded to guix-patches@gnu.org:
bug#30950; Package guix-patches. (Fri, 06 Apr 2018 04:24:02 GMT) (full text, mbox, link).


Message #23 received at 30950@debbugs.gnu.org (full text, mbox, reply):

From: Carlo Zancanaro <carlo@zancanaro.id.au>
To: Ludovic Courtès <ludo@gnu.org>
Cc: 30950@debbugs.gnu.org
Subject: Re: [bug#30950] [PATCH shepherd]: Update required guile version, and remove some hacks
Date: Fri, 06 Apr 2018 14:23:25 +1000
[Message part 1 (text/plain, inline)]
Hey Ludo!

On Thu, Mar 29 2018, Ludovic Courtès wrote:
>> * modules/shepherd.scm (main): Remove SIGALRM hack for guile <= 
>> 2.0.9.
>>
>> ...
>
> Unfortunately I think the problem remains.  That’s one of the 
> reasons for using signalfd(2).

I must not understand this problem. Can you explain what the 
problem is, and how this solves it? Reading the linked email 
didn't help me to understand. I've read a number of other things 
about Guile and how it handles signals and they haven't helped me 
to understand, either.

> Can you create an account on Savannah so I can add you to the 
> group and let you push the first two patches?  :-)

I have an activated account, finally! I'm czan there.

Carlo
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches@gnu.org:
bug#30950; Package guix-patches. (Fri, 06 Apr 2018 09:32:02 GMT) (full text, mbox, link).


Message #26 received at 30950@debbugs.gnu.org (full text, mbox, reply):

From: ludo@gnu.org (Ludovic Courtès)
To: Carlo Zancanaro <carlo@zancanaro.id.au>
Cc: 30950@debbugs.gnu.org
Subject: Re: [bug#30950] [PATCH shepherd]: Update required guile version, and remove some hacks
Date: Fri, 06 Apr 2018 11:31:14 +0200
Hi Carlo,

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> On Thu, Mar 29 2018, Ludovic Courtès wrote:
>>> * modules/shepherd.scm (main): Remove SIGALRM hack for guile <=
>>> 2.0.9.
>>>
>>> ...
>>
>> Unfortunately I think the problem remains.  That’s one of the
>> reasons for using signalfd(2).
>
> I must not understand this problem. Can you explain what the problem
> is, and how this solves it? Reading the linked email didn't help me to
> understand. I've read a number of other things about Guile and how it
> handles signals and they haven't helped me to understand, either.

It’s a limitation/bug in how Guile handles signals.  Scheme signal
handlers are added to a queue of “system asyncs” (info "(guile)
Asyncs").  As the name implies, those asyncs get executed
asynchronously; this is what the ‘handle-interrupts’ instructions that
we see here are for:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,compile (lambda (x) (+ 1 x))
Disassembly of <unnamed function> at #xe8:

   0    (assert-nargs-ee/locals 1 1)    ;; 2 slots (0 args)   at (unknown file):139:9
   1    (make-non-immediate 0 39)       ;; #<procedure 12fea68 at <unknown port>:139:9 (x)>
   3    (handle-interrupts)             
   4    (return-values 2)               ;; 1 value


Disassembly of <unnamed function> at #xfc:

   0    (assert-nargs-ee/locals 2 0)    ;; 2 slots (1 arg)    at (unknown file):139:9
   1    (add/immediate 0 0 1)                                 at (unknown file):139:21
   2    (handle-interrupts)             
   3    (return-values 2)               ;; 1 value


Disassembly of <unnamed function> at #x10c:

   0    (assert-nargs-ee/locals 1 1)    ;; 2 slots (0 args)   at (unknown file):139:21
   1    (static-patch! 32 -5)           
   4    (make-short-immediate 0 2052)   ;; #<unspecified>
   5    (return-values 2)               ;; 1 value
--8<---------------cut here---------------end--------------->8---

The problem is that if you have a loop around the ‘select’ syscall, you
could have a situation like this:

  1. You receive SIGCHLD; an async is queued by the C signal handler in
     libguile, and select(2) exits with EINTR.

  2. The Scheme code that called the ‘select’ procedure runs and loops
     back to the ‘select’ call.

  3. We’re now back in select(2) but we haven’t executed our signal
     handler (async), and we know it won’t run until we’ve returned from
     select(2), which could be hours away.

That’s roughly the story.  I would need to “page it in” again to think
about what can be done.

>> Can you create an account on Savannah so I can add you to the group
>> and let you push the first two patches?  :-)
>
> I have an activated account, finally! I'm czan there.

Awesome, you’re a member now, you can unleash your hacking power.  :-)

Cheers,
Ludo’.




Added tag(s) fixed. Request was from ludo@gnu.org (Ludovic Courtès) to control@debbugs.gnu.org. (Sat, 28 Apr 2018 21:28:02 GMT) (full text, mbox, link).


bug closed, send any further explanations to 30950@debbugs.gnu.org and Carlo Zancanaro <carlo@zancanaro.id.au> Request was from ludo@gnu.org (Ludovic Courtès) to control@debbugs.gnu.org. (Sat, 28 Apr 2018 21:28:02 GMT) (full text, mbox, link).


bug archived. Request was from Debbugs Internal Request <help-debbugs@gnu.org> to internal_control@debbugs.gnu.org. (Sun, 27 May 2018 11:24:05 GMT) (full text, mbox, link).


Send a report that this bug log contains spam.


debbugs.gnu.org maintainers <help-debbugs@gnu.org>. Last modified: Mon Nov 4 22:05:54 2024; Machine Name: wallace-server

GNU bug tracking system

Debbugs is free software and licensed under the terms of the GNU Public License version 2. The current version can be obtained from https://bugs.debian.org/debbugs-source/.

Copyright © 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson, 2005-2017 Don Armstrong, and many other contributors.