[shepherd] branch main updated: repl: Do not attempt to enter debugger on error.

[Ludovic Courtès]

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

civodul pushed a commit to branch main
in repository shepherd.

The following commit(s) were added to refs/heads/main by this push:
     new 69b6386  repl: Do not attempt to enter debugger on error.
69b6386 is described below

commit 69b63861b2fed0238086018fdde3f79c68ea2aea
Author: Ludovic Courtès <ludo@gnu.org>
AuthorDate: Wed Feb 21 18:53:53 2024 +0100

    repl: Do not attempt to enter debugger on error.
    
    Fixes the bug reported at
    <https://lists.gnu.org/archive/html/guix-devel/2024-01/msg00064.html>.
    
    * modules/shepherd/service/repl.scm (start-repl-without-debugger): New
    procedure.
    (run-client-repl): Use it instead of ‘start-repl’.
    * tests/services/repl.sh: Add test.
    
    Reported-by: Justin Veilleux <terramorpha@cock.li>
---
 NEWS                              |  9 +++++++++
 modules/shepherd/service/repl.scm | 20 ++++++++++++++++----
 tests/services/repl.sh            | 30 ++++++++++++++++++++++++++++--
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index ea40b08..0babb66 100644
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,15 @@ In runc environments (among others), reboot(RB_DISABLE_CAD) 
returns ENOSYS,
 which would lead shepherd to fail to start.  This would prevent the use of
 shepherd in some containerized environments such as those of GitLab-CI.
 
+** REPL service no longer attempts to enter debugger upon error
+
+The REPL service would spawn a regular REPL that enters a debugger (or
+“recursive prompt”) by default.  While this is a great feature, it could
+easily render the shepherd REPL unusable because the continuation of the
+debugger prompt could not always be suspended—see the thread at
+https://lists.gnu.org/archive/html/guix-devel/2024-01/msg00064.html.  To avoid
+that, the REPL now simply displays a backtrace upon error.
+
 * Changes in 0.10.3
 
 ** Fix a bug that could lead shepherd to hang after loading replacements
diff --git a/modules/shepherd/service/repl.scm 
b/modules/shepherd/service/repl.scm
index d7bc65a..9179672 100644
--- a/modules/shepherd/service/repl.scm
+++ b/modules/shepherd/service/repl.scm
@@ -1,5 +1,5 @@
 ;; repl.scm -- Read-eval-print loop.
-;; Copyright (C) 2023 Ludovic Courtès <ludo@gnu.org>
+;; Copyright (C) 2023-2024 Ludovic Courtès <ludo@gnu.org>
 ;;
 ;; This file is part of the GNU Shepherd.
 ;;
@@ -23,9 +23,9 @@
   #:use-module (fibers)
   #:use-module (fibers channels)
   #:use-module (fibers io-wakeup)
-  #:autoload   (system repl repl) (start-repl)
+  #:autoload   (system repl common) (make-repl repl-option-set!)
+  #:autoload   (system repl repl) (run-repl)
   #:use-module (ice-9 match)
-  #:use-module (oop goops)
   #:export (default-repl-socket-file
             repl-service))
 
@@ -73,6 +73,12 @@ socket.  Use @var{id} to create the service name."
     (module-set! module 'sleep (@ (fibers) sleep)) ;avoid that pitfall
     module))
 
+(define (start-repl-without-debugger)
+  "Start a REPL that doesn't enter the debugger when an exception is thrown."
+  (let ((repl (make-repl (current-language) #f)))
+    (repl-option-set! repl 'on-error 'backtrace)
+    (run-repl repl)))
+
 (define (run-client-repl service client)
   "Return a REPL on @var{client}, a socket.  When the REPL terminates or
 crashes, stop @var{service}."
@@ -86,7 +92,13 @@ crashes, stop @var{service}."
          (lambda ()
            (set-current-module user-module)
            (with-fluids ((*repl-stack* '()))
-             (start-repl))))))
+             ;; The default REPL behavior is to enter the debugger upon error.
+             ;; This is great but it's not always possible in a Fibers context
+             ;; because there might be continuation barriers such as C frames
+             ;; on the stack.  Thus, to be on the safe side, do not start the
+             ;; debugger upon error.  See
+             ;; 
<https://lists.gnu.org/archive/html/guix-devel/2024-01/msg00064.html>.
+             (start-repl-without-debugger))))))
     (lambda args
       (local-output (l10n "Uncaught REPL exception: ~s.") args)))
   (stop-service service))
diff --git a/tests/services/repl.sh b/tests/services/repl.sh
index 597858a..0c807c5 100644
--- a/tests/services/repl.sh
+++ b/tests/services/repl.sh
@@ -1,5 +1,5 @@
 # GNU Shepherd --- Test monitoring service.
-# Copyright © 2023 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2023-2024 Ludovic Courtès <ludo@gnu.org>
 #
 # This file is part of the GNU Shepherd.
 #
@@ -23,12 +23,13 @@ socket="t-socket-$$"
 conf="t-conf-$$"
 log="t-log-$$"
 pid="t-pid-$$"
+witness="t-repl-witness-$$"
 repl_socket="$PWD/repl-socket-$$"
 
 herd="herd -s $socket"
 
 trap "cat $log || true;
-      rm -f $socket $repl_socket $conf $log;
+      rm -f $socket $repl_socket $conf $log $witness;
       test -f $pid && kill \`cat $pid\` || true; rm -f $pid" EXIT
 
 cat > "$conf" <<EOF
@@ -120,6 +121,31 @@ grep "heap:" "$log"
 
 $herd log monitoring | grep "heap:"
 
+# What if we evaluate code that raises an exception?  Does the REPL remain
+# functional?  It used to enter an infinite loop while spawning the debugger,
+# due to non-suspendable continuations:
+# <https://lists.gnu.org/archive/html/guix-devel/2024-01/msg00064.html>.
+rm -f "$witness"
+guile -c '
+(alarm 10)
+(let ((sock (socket AF_UNIX SOCK_STREAM 0)))
+  (connect sock PF_UNIX "'$repl_socket'")
+  (format #t "connected!~%> ")
+  (display "this-is-unbound-and-thus-throws" sock)
+  (newline sock)
+  (display (object->string (quote (open-output-file "'$witness'"))) sock)
+  (newline sock)
+  (display ",q\n" sock)
+
+  (let loop ()
+    (define chr (read-char sock))
+    (unless (eof-object? chr)
+      (display chr)
+      (when (eq? chr #\newline)
+       (display "> "))
+      (loop))))'
+test -f "$witness"
+
 $herd stop repl
 $herd status repl | grep "stopped"