You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by gn...@apache.org on 2025/04/29 22:10:08 UTC

(felix-dev) branch fix-interruption-nested-shells created (now d9b07f8d66)

This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a change to branch fix-interruption-nested-shells
in repository https://gitbox.apache.org/repos/asf/felix-dev.git


      at d9b07f8d66 Fix interruption handling in nested shells

This branch includes the following new commits:

     new d9b07f8d66 Fix interruption handling in nested shells

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(felix-dev) 01/01: Fix interruption handling in nested shells

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a commit to branch fix-interruption-nested-shells
in repository https://gitbox.apache.org/repos/asf/felix-dev.git

commit d9b07f8d6614ec80cb2703b11fad30357b03e581
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Wed Apr 30 00:09:47 2025 +0200

    Fix interruption handling in nested shells
    
    This change modifies the Shell.sh method to properly handle interruption signals
    in nested shells by clearing and restoring the current pipe. When a user starts a
    shell session, then runs the 'sh' command to create a nested shell, and then runs
    a command like 'ttop' in that nested shell, pressing Ctrl+C now properly interrupts
    the command.
    
    This complements PR #411 which made the Pipe.setCurrentPipe method public to enable
    this fix. Together, these changes resolve the issue reported in jline/jline3#1143
    where interruption exceptions were not working for child sessions.
    
    The fix follows the same pattern implemented in the Posix.runShell method, ensuring
    consistent behavior across different shell creation methods.
---
 commit-message.txt                                 | 14 +++++++++
 gogo/jline/pom.xml                                 |  2 +-
 .../java/org/apache/felix/gogo/jline/Posix.java    | 35 +++++++++++++++++++++-
 .../java/org/apache/felix/gogo/jline/Shell.java    | 21 ++++++++++++-
 4 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/commit-message.txt b/commit-message.txt
new file mode 100644
index 0000000000..8232678fde
--- /dev/null
+++ b/commit-message.txt
@@ -0,0 +1,14 @@
+Fix interruption handling in nested shells
+
+This change modifies the Shell.sh method to properly handle interruption signals
+in nested shells by clearing and restoring the current pipe. When a user starts a
+shell session, then runs the 'sh' command to create a nested shell, and then runs
+a command like 'ttop' in that nested shell, pressing Ctrl+C now properly interrupts
+the command.
+
+This complements PR #411 which made the Pipe.setCurrentPipe method public to enable
+this fix. Together, these changes resolve the issue reported in jline/jline3#1143
+where interruption exceptions were not working for child sessions.
+
+The fix follows the same pattern implemented in the Posix.runShell method, ensuring
+consistent behavior across different shell creation methods.
diff --git a/gogo/jline/pom.xml b/gogo/jline/pom.xml
index 9056afba83..cf8827999d 100644
--- a/gogo/jline/pom.xml
+++ b/gogo/jline/pom.xml
@@ -58,7 +58,7 @@
         <dependency>
             <groupId>org.apache.felix</groupId>
             <artifactId>org.apache.felix.gogo.runtime</artifactId>
-            <version>1.1.0</version>
+            <version>1.1.7-SNAPSHOT</version>
         </dependency>
         <dependency>
             <groupId>org.jline</groupId>
diff --git a/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java b/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java
index 9ead00c263..a954bf0311 100644
--- a/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java
+++ b/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java
@@ -71,6 +71,7 @@ import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.apache.felix.gogo.runtime.Pipe;
 import org.apache.felix.service.command.Process;
 import org.apache.felix.gogo.jline.Shell.Context;
 import org.apache.felix.service.command.CommandProcessor;
@@ -808,9 +809,26 @@ public class Posix {
         new Thread(() -> runShell(session, terminal), terminal.getName() + " shell").start();
     }
 
+    /**
+     * Run a shell in a new terminal.
+     *
+     * This method has been modified to fix an issue with interruption handling in nested shells.
+     * The fix clears the current pipe before creating a child shell and restores it afterward,
+     * ensuring that when Ctrl+C is pressed in a child shell (e.g., when running "sh" followed by "ttop"),
+     * the interruption signal is properly propagated to the child process.
+     *
+     * See: https://github.com/jline/jline3/issues/1143
+     *
+     * @param session The parent command session
+     * @param terminal The terminal to use for the new shell
+     */
     private void runShell(CommandSession session, Terminal terminal) {
         InputStream in = terminal.input();
         OutputStream out = terminal.output();
+        
+        // Save the current pipe and clear it before creating a child shell
+        Pipe currentPipe = Pipe.setCurrentPipe(null);
+
         CommandSession newSession = processor.createSession(in, out, out);
         newSession.put(Shell.VAR_TERMINAL, terminal);
         newSession.put(".tmux", session.get(".tmux"));
@@ -822,11 +840,26 @@ public class Posix {
                 terminal.close();
             }
         };
+
+        // Register a signal handler for INT signal to properly propagate interruption
+        Terminal.SignalHandler prevIntHandler = terminal.handle(Terminal.Signal.INT, signal -> {
+            // Propagate the interrupt to the current thread
+            Thread.currentThread().interrupt();
+        });
+        
         try {
-            new Shell(context, processor).gosh(newSession, new String[]{"--login"});
+            new Shell(context, processor).gosh(newSession, new String[] {"--login"});
         } catch (Exception e) {
             e.printStackTrace();
         } finally {
+            // Restore the previous signal handler
+            if (prevIntHandler != null) {
+                terminal.handle(Terminal.Signal.INT, prevIntHandler);
+            }
+            
+            // Restore the previous pipe
+            Pipe.setCurrentPipe(currentPipe);
+
             try {
                 terminal.close();
             } catch (IOException e) {
diff --git a/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Shell.java b/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Shell.java
index eaf0901102..d3da4b37e2 100644
--- a/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Shell.java
+++ b/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Shell.java
@@ -46,6 +46,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.felix.gogo.runtime.Closure;
 import org.apache.felix.gogo.runtime.CommandProxy;
 import org.apache.felix.gogo.runtime.CommandSessionImpl;
+import org.apache.felix.gogo.runtime.Pipe;
 import org.apache.felix.service.command.Job;
 import org.apache.felix.service.command.Job.Status;
 import org.apache.felix.gogo.runtime.Reflective;
@@ -440,6 +441,16 @@ public class Shell {
 
     private Object runShell(final CommandSession session, Terminal terminal,
                             LineReader reader) throws InterruptedException {
+        Pipe currentPipe = Pipe.setCurrentPipe(null);
+        try {
+            return doRunShell(session, terminal, reader);
+        } finally {
+            Pipe.setCurrentPipe(currentPipe);
+        }
+    }
+
+    private Object doRunShell(final CommandSession session, Terminal terminal,
+                            LineReader reader) throws InterruptedException {
         AtomicBoolean reading = new AtomicBoolean();
         session.setJobListener((job, previous, current) -> {
             if (previous == Status.Background || current == Status.Background
@@ -555,7 +566,15 @@ public class Shell {
 
     @Descriptor("start a new shell")
     public Object sh(final CommandSession session, String[] argv) throws Exception {
-        return gosh(session, argv);
+        // Save the current pipe and clear it before creating a child shell
+        // This ensures that interruption signals are properly propagated to the child shell
+        Pipe currentPipe = Pipe.setCurrentPipe(null);
+        try {
+            return gosh(session, argv);
+        } finally {
+            // Restore the previous pipe
+            Pipe.setCurrentPipe(currentPipe);
+        }
     }
 
     private void shutdown() throws Exception {