Skip to content

Commit 87bf8b3

Browse files
authored
Fix macOS hang in pipe operations by removing PTY terminal usage (#1382)
* Fix macOS hang in pipe operations by removing PTY terminal usage - Remove PTY terminal creation in CommandOutputStream.open() method - Use simple Java streams instead of TerminalBuilder for pipe operations - Add unit test to verify pipe operations don't hang with timeout - Clean up unused imports (TerminalBuilder, Attributes, InputFlag, etc.) Fixes #1361 and #1360 where pipe operations would hang on macOS due to PTY terminal creation. The fix follows the recommendation from PR #1367 discussion to use simple Java piped streams instead of PTY terminals for command pipes. The CommandOutputStream now reuses the original terminal for input and uses redirected streams for output, avoiding the problematic PTY terminal creation that caused hangs on BSD/macOS platforms. * Fix variable assignment test syntax - Use correct syntax 'variable=command' without spaces around equals - Test now properly triggers CommandOutputStream.open() method - Verifies that variable assignments don't hang on macOS with timeout
1 parent 0ee349c commit 87bf8b3

File tree

2 files changed

+68
-28
lines changed

2 files changed

+68
-28
lines changed

console/src/main/java/org/jline/console/impl/SystemRegistryImpl.java

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,10 @@
88
*/
99
package org.jline.console.impl;
1010

11-
import java.io.ByteArrayInputStream;
1211
import java.io.ByteArrayOutputStream;
1312
import java.io.File;
1413
import java.io.FileOutputStream;
1514
import java.io.IOException;
16-
import java.io.InputStream;
1715
import java.io.OutputStream;
1816
import java.io.PrintStream;
1917
import java.nio.file.Path;
@@ -40,14 +38,9 @@
4038
import org.jline.reader.impl.completer.NullCompleter;
4139
import org.jline.reader.impl.completer.StringsCompleter;
4240
import org.jline.reader.impl.completer.SystemCompleter;
43-
import org.jline.terminal.Attributes;
44-
import org.jline.terminal.Attributes.InputFlag;
4541
import org.jline.terminal.Terminal;
46-
import org.jline.terminal.TerminalBuilder;
4742
import org.jline.utils.*;
4843

49-
import static org.jline.keymap.KeyMap.ctrl;
50-
5144
/**
5245
* Aggregate command registries.
5346
*
@@ -494,36 +487,24 @@ public void open(boolean redirectColor) throws IOException {
494487
PrintStream out = new PrintStream(outputStream);
495488
System.setOut(out);
496489
System.setErr(out);
497-
String input = ctrl('X') + "q";
498-
InputStream in = new ByteArrayInputStream(input.getBytes());
499-
Attributes attrs = new Attributes();
500-
if (OSUtils.IS_WINDOWS) {
501-
attrs.setInputFlag(InputFlag.IGNCR, true);
502-
}
503-
try {
504-
terminal = TerminalBuilder.builder()
505-
.streams(in, outputStream)
506-
.attributes(attrs)
507-
.type((redirectColor ? Terminal.TYPE_DUMB_COLOR : Terminal.TYPE_DUMB))
508-
.build();
509-
this.commandSession = new CommandRegistry.CommandSession(terminal, terminal.input(), out, out);
510-
redirecting = true;
511-
} catch (IOException e) {
512-
reset();
513-
throw e;
514-
}
490+
491+
// Use simple streams instead of creating a PTY terminal to avoid hangs on macOS
492+
// Create a command session that uses the original terminal for input but redirected streams for output
493+
this.commandSession = new CommandRegistry.CommandSession(origTerminal, origTerminal.input(), out, out);
494+
redirecting = true;
515495
}
516496

517497
public void close() {
518498
if (!redirecting) {
519499
return;
520500
}
521501
try {
522-
terminal.flush();
502+
// Flush the original terminal since we're using it for input
503+
origTerminal.flush();
523504
if (outputStream instanceof ByteArrayOutputStream) {
524505
output = outputStream.toString();
525506
}
526-
terminal.close();
507+
// No need to close a separate terminal since we're reusing the original one
527508
} catch (Exception e) {
528509
// ignore
529510
}
@@ -538,7 +519,6 @@ private void reset() {
538519
outputStream = null;
539520
System.setOut(origOut);
540521
System.setErr(origErr);
541-
terminal = null;
542522
terminal = origTerminal;
543523
PrintStream ps = new PrintStream(terminal.output());
544524
this.commandSession = new CommandRegistry.CommandSession(terminal, terminal.input(), ps, ps);

console/src/test/java/org/jline/console/impl/SystemRegistryImplTest.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
import java.util.List;
1717
import java.util.Map;
1818
import java.util.Set;
19+
import java.util.concurrent.CompletableFuture;
20+
import java.util.concurrent.TimeUnit;
21+
import java.util.concurrent.TimeoutException;
1922
import java.util.function.Supplier;
2023

2124
import org.jline.builtins.ConfigurationPath;
@@ -79,6 +82,59 @@ public void testOverrideBuiltinCommand() throws Exception {
7982
assertEquals("Custom exit command executed", output.toString().trim());
8083
}
8184

85+
/**
86+
* Test that variable assignment operations don't hang on macOS.
87+
*
88+
* This test verifies the fix for issues #1361 and #1360 where variable assignments
89+
* would hang on macOS due to PTY terminal creation in CommandOutputStream.
90+
* The fix removes PTY terminal usage and uses simple Java streams instead.
91+
*
92+
* Variable assignments trigger CommandOutputStream.open() which previously created
93+
* PTY terminals that could hang on BSD/macOS platforms.
94+
*/
95+
@Test
96+
public void testVariableAssignmentDoesNotHang() throws Exception {
97+
// Add a test command that outputs some text
98+
TestCommandRegistry echoRegistry = new TestCommandRegistry(output);
99+
echoRegistry.addCommand("echo", (input) -> {
100+
if (input.args().length > 0) {
101+
StringBuilder sb = new StringBuilder();
102+
for (Object arg : input.args()) {
103+
if (sb.length() > 0) sb.append(" ");
104+
sb.append(arg.toString());
105+
}
106+
// Print to System.out which will be captured by CommandOutputStream
107+
System.out.print(sb.toString());
108+
}
109+
return null;
110+
});
111+
112+
registry.setCommandRegistries(echoRegistry);
113+
114+
// Test variable assignment with a timeout to ensure it doesn't hang
115+
CompletableFuture<Void> future = CompletableFuture.runAsync(() -> {
116+
try {
117+
// This command uses variable assignment which triggers CommandOutputStream.open()
118+
// and previously could hang on macOS due to PTY terminal creation
119+
registry.execute("result=echo hello world");
120+
} catch (Exception e) {
121+
throw new RuntimeException(e);
122+
}
123+
});
124+
125+
try {
126+
// If the operation hangs, this will throw TimeoutException
127+
future.get(5, TimeUnit.SECONDS);
128+
} catch (TimeoutException e) {
129+
future.cancel(true);
130+
throw new AssertionError(
131+
"Variable assignment operation hung - this indicates the macOS hang bug is present", e);
132+
}
133+
134+
// If we get here, the operation completed without hanging
135+
// The exact output doesn't matter as much as the fact that it didn't hang
136+
}
137+
82138
/**
83139
* A test command registry that provides a custom implementation of the "exit" command.
84140
*/
@@ -92,6 +148,10 @@ public TestCommandRegistry(StringBuilder output) {
92148
commandExecute.put("exit", new CommandMethods(this::exit, this::defaultCompleter));
93149
}
94150

151+
public void addCommand(String name, java.util.function.Function<CommandInput, Object> executor) {
152+
commandExecute.put(name, new CommandMethods(executor, this::defaultCompleter));
153+
}
154+
95155
private Object exit(CommandInput input) {
96156
output.append("Custom exit command executed");
97157
return null;

0 commit comments

Comments
 (0)