fix: terminal.close() blocks when pump thread is reading stdin (#1911)

* fix: terminal.close() blocks when pump thread is reading stdin (#1909)

On macOS, a pump thread blocked in a native read() on a tty can prevent
tcsetattr() from completing, causing terminal.close() to hang until the
user presses ENTER. Fix by switching to non-canonical mode with
VMIN=0/VTIME=1 before shutdown, which forces the blocked read() to time
out within 100 ms so the pump thread can exit cleanly before original
terminal attributes are restored.

Also improve NonBlockingInputStreamImpl and NonBlockingReaderImpl
shutdown: set threadIsReading=false, interrupt the thread, and join
with a timeout so close() reliably stops the pump thread.

* fix: ensure pump thread shutdown is robust on close

- AbstractUnixSysTerminal: wrap VMIN/VTIME unblock sequence in
  try-finally so input.shutdown() runs even if doGetAttributes()
  or doSetAttributes() throws
- NonBlockingInputStreamImpl/ReaderImpl: close the wrapped stream
  before shutdown/join so the native read() is unblocked before
  we wait for the pump thread to exit

* refactor: extract NonCloseable stream wrappers and add lifecycle tests

Extract NonCloseableInputStream and NonCloseableOutputStream from
DumbTerminalProvider into org.jline.utils for reuse. Apply them in
AbstractUnixSysTerminal to prevent closing shared FileDescriptors
(stdin/stdout/stderr) when the terminal is closed.

Add timeout logging to pump thread shutdown in NonBlockingInputStreamImpl
and NonBlockingReaderImpl. Add comprehensive tests for stream wrapper
behavior and pump thread lifecycle.

* fix: wrap ExecPty system streams with NonCloseable wrappers

ExecPty.doGetSlaveInput() and getSlaveOutput() returned raw
FileInputStream/FileOutputStream on shared FileDescriptors when used
as a system terminal. Closing the terminal would close the shared
FDs, breaking System.in/out/err for the rest of the JVM.

Wrap the system stream cases with NonCloseableInputStream and
NonCloseableOutputStream, matching AbstractUnixSysTerminal and
DumbTerminalProvider. Device-file paths are left unwrapped since
those are non-shared FDs.

* refactor: remove VMIN/VTIME unblock sequence from doClose()

The VMIN/VTIME trick cannot reliably unblock a native read() that is
already in progress — tcsetattr only affects the next read call, not
the current one. The NonCloseable wrappers are the actual fix: they
prevent FileDescriptor.close0() from being called, which eliminates
the macOS deadlock. The pump thread exits on its next iteration once
the current read completes naturally.

* fix: reduce shutdown join timeout and prevent double-wait

Reduce the thread join timeout from 500ms to 50ms — the thread either
exits promptly (when waiting) or is stuck in a native read (where no
timeout helps). Setting thread = null after join prevents the second
shutdown() call in the close chain from waiting again.

* refactor: extract PumpThread to deduplicate thread lifecycle management

NonBlockingInputStreamImpl and NonBlockingReaderImpl had identical
startReadingThreadIfNeeded(), shutdown(), and thread management fields.
Extract into a shared PumpThread helper that both classes delegate to.

* refactor: move run loop into PumpThread to further reduce duplication

The run() method was still duplicated between NonBlockingInputStreamImpl
and NonBlockingReaderImpl. Move the loop into PumpThread.runLoop() with
IoReader and ResultHandler callbacks. Each Impl class now has a one-liner
run() that delegates to pump.runLoop().

* fix: address SonarCloud issues in PumpThread

Store lock as a field instead of passing as method parameter (S2445).
Use notifyAll instead of notify (S2446). Re-interrupt on
InterruptedException in runLoop (S2142).

Author: gnodet

terminal/src/main/java/org/jline/terminal/impl/AbstractUnixSysTerminal.java

@@ -31,6 +31,8 @@
 import org.jline.utils.NonBlocking;
 import org.jline.utils.NonBlockingInputStream;
 import org.jline.utils.NonBlockingReader;
+import org.jline.utils.NonCloseableInputStream;
+import org.jline.utils.NonCloseableOutputStream;
 import org.jline.utils.ShutdownHooks;
 import org.jline.utils.ShutdownHooks.Task;
 
@@ -50,6 +52,12 @@
  * <pre>
  *   Terminal → AbstractTerminal → AbstractUnixSysTerminal → subclass → native call
  * </pre>
+ *
+ * <p><strong>Important:</strong> the underlying system streams ({@code FileDescriptor.in},
+ * {@code FileDescriptor.out}/{@code err}) are wrapped in {@link NonCloseableInputStream} /
+ * {@link NonCloseableOutputStream}. Closing the terminal will shut down the pump thread and
+ * release resources, but will <em>not</em> close the shared file descriptors. This prevents
+ * breaking {@code System.in}/{@code System.out} for the rest of the JVM.</p>
  */
 public abstract class AbstractUnixSysTerminal extends AbstractTerminal {
 
@@ -87,7 +95,8 @@ protected AbstractUnixSysTerminal(
         this.originalAttributes = originalAttributes;
         this.nativeSignals = nativeSignals;
 
-        this.input = NonBlocking.nonBlocking(getName(), new FileInputStream(FileDescriptor.in));
+        this.input =
+                NonBlocking.nonBlocking(getName(), new NonCloseableInputStream(new FileInputStream(FileDescriptor.in)));
         FileDescriptor outFd;
         if (systemStream == SystemStream.Output) {
             outFd = FileDescriptor.out;
@@ -96,7 +105,7 @@ protected AbstractUnixSysTerminal(
         } else {
             throw new IllegalArgumentException("Invalid system stream for output: " + systemStream);
         }
-        this.output = new FastBufferedOutputStream(new FileOutputStream(outFd));
+        this.output = new FastBufferedOutputStream(new NonCloseableOutputStream(new FileOutputStream(outFd)));
         this.reader = NonBlocking.nonBlocking(getName(), input, inputEncoding());
         this.writer = new PrintWriter(new OutputStreamWriter(output, outputEncoding()));
 
@@ -231,9 +240,13 @@ protected void doClose() throws IOException {
                 super.doClose();
             } finally {
                 try {
-                    doSetAttributes(originalAttributes);
+                    input.close();
                 } finally {
-                    reader.close();
+                    try {
+                        doSetAttributes(originalAttributes);
+                    } finally {
+                        reader.close();
+                    }
                 }
             }
         }

terminal/src/main/java/org/jline/terminal/impl/DumbTerminalProvider.java

@@ -11,8 +11,6 @@
 import java.io.FileDescriptor;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
-import java.io.FilterInputStream;
-import java.io.FilterOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -24,6 +22,8 @@
 import org.jline.terminal.TerminalBuilder;
 import org.jline.terminal.spi.SystemStream;
 import org.jline.terminal.spi.TerminalProvider;
+import org.jline.utils.NonCloseableInputStream;
+import org.jline.utils.NonCloseableOutputStream;
 
 /**
  * Terminal provider implementation for dumb terminals.
@@ -127,35 +127,4 @@ public int systemStreamWidth(SystemStream stream) {
     public String toString() {
         return "TerminalProvider[" + name() + "]";
     }
-
-    /**
-     * Wrapper that prevents closing the underlying input stream.
-     * Used for system streams (System.in) to prevent closing the FileDescriptor.
-     */
-    private static class NonCloseableInputStream extends FilterInputStream {
-        NonCloseableInputStream(InputStream in) {
-            super(in);
-        }
-
-        @Override
-        public void close() throws IOException {
-            // Do not close the underlying stream
-        }
-    }
-
-    /**
-     * Wrapper that prevents closing the underlying output stream.
-     * Used for system streams (System.out/err) to prevent closing the FileDescriptor.
-     */
-    private static class NonCloseableOutputStream extends FilterOutputStream {
-        NonCloseableOutputStream(OutputStream out) {
-            super(out);
-        }
-
-        @Override
-        public void close() throws IOException {
-            // Flush but do not close the underlying stream
-            flush();
-        }
-    }
 }

terminal/src/main/java/org/jline/terminal/impl/exec/ExecPty.java

@@ -31,6 +31,8 @@
 import org.jline.terminal.spi.Pty;
 import org.jline.terminal.spi.SystemStream;
 import org.jline.terminal.spi.TerminalProvider;
+import org.jline.utils.NonCloseableInputStream;
+import org.jline.utils.NonCloseableOutputStream;
 import org.jline.utils.OSUtils;
 
 import static org.jline.utils.ExecHelper.exec;
@@ -154,15 +156,17 @@ public OutputStream getMasterOutput() {
 
     @Override
     protected InputStream doGetSlaveInput() throws IOException {
-        return systemStream != null ? new FileInputStream(FileDescriptor.in) : new FileInputStream(getName());
+        return systemStream != null
+                ? new NonCloseableInputStream(new FileInputStream(FileDescriptor.in))
+                : new FileInputStream(getName());
     }
 
     @Override
     public OutputStream getSlaveOutput() throws IOException {
         return systemStream == SystemStream.Output
-                ? new FileOutputStream(FileDescriptor.out)
+                ? new NonCloseableOutputStream(new FileOutputStream(FileDescriptor.out))
                 : systemStream == SystemStream.Error
-                        ? new FileOutputStream(FileDescriptor.err)
+                        ? new NonCloseableOutputStream(new FileOutputStream(FileDescriptor.err))
                         : new FileOutputStream(getName());
     }
 

terminal/src/main/java/org/jline/utils/NonBlockingInputStreamImpl.java

@@ -31,10 +31,8 @@ public class NonBlockingInputStreamImpl extends NonBlockingInputStream {
     private int b = READ_EXPIRED; // Recently read byte
 
     private String name;
-    private boolean threadIsReading = false;
     private IOException exception = null;
-    private long threadDelay = 60 * 1000;
-    private Thread thread;
+    private final PumpThread pump;
 
     /**
      * Creates a <code>NonBlockingReader</code> out of a normal blocking
@@ -44,40 +42,25 @@ public class NonBlockingInputStreamImpl extends NonBlockingInputStream {
      * @param name The stream name
      * @param in The reader to wrap
      */
+    @SuppressWarnings("this-escape")
     public NonBlockingInputStreamImpl(String name, InputStream in) {
         this.in = in;
         this.name = name;
+        this.pump = new PumpThread(this, 60_000);
     }
 
-    private synchronized void startReadingThreadIfNeeded() {
-        if (thread == null) {
-            thread = new Thread(this::run);
-            thread.setName(name + " non blocking reader thread");
-            thread.setDaemon(true);
-            thread.start();
-        }
-    }
-
-    /**
-     * Shuts down the thread that is handling blocking I/O. Note that if the
-     * thread is currently blocked waiting for I/O it will not actually
-     * shut down until the I/O is received.
-     */
-    public synchronized void shutdown() {
-        if (thread != null) {
-            notify();
-        }
+    public void shutdown() {
+        pump.shutdown();
     }
 
     @Override
     public void close() throws IOException {
-        /*
-         * The underlying input stream is closed first. This means that if the
-         * I/O thread was blocked waiting on input, it will be woken for us.
-         */
-        super.close(); // Mark as closed in base class
-        in.close();
-        shutdown();
+        super.close();
+        try {
+            in.close();
+        } finally {
+            pump.shutdown();
+        }
     }
 
     /**
@@ -110,15 +93,15 @@ public synchronized int read(long timeout, boolean isPeek) throws IOException {
          */
         if (b >= -1) {
             assert exception == null;
-        } else if (!isPeek && timeout <= 0L && !threadIsReading) {
+        } else if (!isPeek && timeout <= 0L && !pump.isReading()) {
             b = in.read();
         } else {
             /*
              * If the thread isn't reading already, then ask it to do so.
              */
-            if (!threadIsReading) {
-                threadIsReading = true;
-                startReadingThreadIfNeeded();
+            if (!pump.isReading()) {
+                pump.setReading(true);
+                pump.startIfNeeded(this::run, name);
                 notifyAll();
             }
 
@@ -166,66 +149,12 @@ public synchronized int read(long timeout, boolean isPeek) throws IOException {
     }
 
     private void run() {
-        Log.debug("NonBlockingInputStream start");
-        boolean needToRead;
-
-        try {
-            while (true) {
-
-                /*
-                 * Synchronize to grab variables accessed by both this thread
-                 * and the accessing thread.
-                 */
-                synchronized (this) {
-                    needToRead = this.threadIsReading;
-
-                    try {
-                        /*
-                         * Nothing to do? Then wait.
-                         */
-                        if (!needToRead) {
-                            wait(threadDelay);
-                        }
-                    } catch (InterruptedException e) {
-                        /* IGNORED */
-                    }
-
-                    needToRead = this.threadIsReading;
-                    if (!needToRead) {
-                        return;
-                    }
-                }
-
-                /*
-                 * We're not shutting down, but we need to read. This cannot
-                 * happen while we are holding the lock (which we aren't now).
-                 */
-                int byteRead = READ_EXPIRED;
-                IOException failure = null;
-                try {
-                    byteRead = in.read();
-                } catch (IOException e) {
-                    failure = e;
-                }
-
-                /*
-                 * Re-grab the lock to update the state.
-                 */
-                synchronized (this) {
+        pump.runLoop(
+                in::read,
+                (value, failure) -> {
                     exception = failure;
-                    b = byteRead;
-                    threadIsReading = false;
-                    notify();
-                }
-            }
-        } catch (Throwable t) {
-            Log.warn("Error in NonBlockingInputStream thread", t);
-        } finally {
-            Log.debug("NonBlockingInputStream shutdown");
-            synchronized (this) {
-                thread = null;
-                threadIsReading = false;
-            }
-        }
+                    b = value;
+                },
+                "NonBlockingInputStream");
     }
 }