diff --git a/src/main/java/com/gregor/jprototerm/Compositor.java b/src/main/java/com/gregor/jprototerm/Compositor.java index 882e8f4..b8bae3f 100644 --- a/src/main/java/com/gregor/jprototerm/Compositor.java +++ b/src/main/java/com/gregor/jprototerm/Compositor.java @@ -114,7 +114,7 @@ public final class Compositor { public void setFont(String family, double size) { metrics.setFont(family, size); paneContentVersion.clear(); - lastWidth = -1.0; // force a redraw on the next frame + layoutVersion++; // recomposite with the new metrics on the next frame } // ---- Tabs and panes ------------------------------------------------------------- @@ -123,8 +123,9 @@ public final class Compositor { return tabs.isEmpty(); } + /** The active pane of the current tab, or {@code null} when no tab is left. */ public TerminalPane activePane() { - return currentTab().activePane(); + return isEmpty() ? null : currentTab().activePane(); } public void navigate(Direction direction) { @@ -173,10 +174,7 @@ public final class Compositor { } public void closeActivePane() { - if (isEmpty()) { - return; - } - TerminalPane active = currentTab().activePane(); + TerminalPane active = activePane(); if (active != null) { closePane(active); } @@ -214,7 +212,8 @@ public final class Compositor { public void newTab() { // Open the new tab in the currently active pane's working directory, so it lands where the // user currently is rather than always in home. - String workingDirectory = isEmpty() ? null : currentTab().activePane().currentWorkingDirectory(); + TerminalPane active = activePane(); + String workingDirectory = active != null ? active.currentWorkingDirectory() : null; tabs.add(new Tab(config, metrics, workingDirectory, this::closePane)); currentTabIndex = tabs.size() - 1; layoutVersion++; @@ -454,11 +453,11 @@ public final class Compositor { private void handleMouseReleased(MouseEvent event) { TerminalPane pane = paneAt(event.getX(), event.getY()); if (pane == null) { - pane = activePane(); + pane = activePane(); // released outside every pane (e.g. mid-drag): route to the active one } MouseButton button = pressedButton == MouseButton.UNKNOWN ? mouseButton(event) : pressedButton; - MouseTarget target = mouseTarget(pane); + MouseTarget target = pane == null ? null : mouseTarget(pane); if (target != null) { send(pane, target, MouseInput.release(button, localX(event.getX(), pane, target), localY(event.getY(), pane, target), modifiers(event)), false, event); } @@ -469,7 +468,10 @@ public final class Compositor { private void handleMouseDragged(MouseEvent event) { TerminalPane pane = paneAt(event.getX(), event.getY()); if (pane == null) { - pane = activePane(); + pane = activePane(); // dragged outside every pane: route to the active one + } + if (pane == null) { + return; } MouseButton button = pressedButton == MouseButton.UNKNOWN ? mouseButton(event) : pressedButton; diff --git a/src/main/java/com/gregor/jprototerm/Daemon.java b/src/main/java/com/gregor/jprototerm/Daemon.java index 92e98fb..a95017f 100644 --- a/src/main/java/com/gregor/jprototerm/Daemon.java +++ b/src/main/java/com/gregor/jprototerm/Daemon.java @@ -9,6 +9,7 @@ import java.nio.channels.ServerSocketChannel; import java.nio.channels.SocketChannel; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.attribute.PosixFilePermissions; @@ -23,6 +24,11 @@ import java.nio.file.attribute.PosixFilePermissions; * (mode 0700), so only the owning user can connect. */ public final class Daemon { + // One request is a single line holding a filesystem path; anything bigger is bogus. + private static final int MAX_REQUEST_BYTES = 4096; + // The accept loop is single-threaded, so a client that stalls must not wedge the daemon. + private static final long READ_TIMEOUT_NANOS = 5_000_000_000L; + private Daemon() { } @@ -31,9 +37,9 @@ public final class Daemon { Path socket = socketPath(); try { Files.createDirectories(socket.getParent()); - trySecureDir(socket.getParent()); + secureDir(socket.getParent()); } catch (IOException ex) { - System.err.println("jprototerm: cannot create socket dir " + socket.getParent() + ": " + ex.getMessage()); + System.err.println("jprototerm: cannot secure socket dir " + socket.getParent() + ": " + ex.getMessage()); return; } @@ -107,25 +113,48 @@ public final class Daemon { manager.openWindow(workingDirectory == null || workingDirectory.isBlank() ? null : workingDirectory.trim()); + connection.configureBlocking(true); connection.write(ByteBuffer.wrap("OK\n".getBytes(StandardCharsets.UTF_8))); } } + // Reads the request line non-blocking with a deadline and a size cap: the accept loop is + // single-threaded, so a client that stalls or never sends a newline must fail the connection + // (an IOException logged by run()) rather than wedge the daemon or grow the buffer unbounded. private static String readLine(SocketChannel channel) throws IOException { + channel.configureBlocking(false); + long deadline = System.nanoTime() + READ_TIMEOUT_NANOS; ByteArrayOutputStream out = new ByteArrayOutputStream(); ByteBuffer buffer = ByteBuffer.allocate(4096); - while (channel.read(buffer) != -1) { - buffer.flip(); - while (buffer.hasRemaining()) { - byte b = buffer.get(); - if (b == '\n') { - return out.toString(StandardCharsets.UTF_8); + while (true) { + int n = channel.read(buffer); + if (n > 0) { + buffer.flip(); + while (buffer.hasRemaining()) { + byte b = buffer.get(); + if (b == '\n') { + return out.toString(StandardCharsets.UTF_8); + } + out.write(b); + if (out.size() > MAX_REQUEST_BYTES) { + throw new IOException("request line too long"); + } + } + buffer.clear(); + } else if (n == -1) { + return out.size() == 0 ? null : out.toString(StandardCharsets.UTF_8); + } else { + if (System.nanoTime() >= deadline) { + throw new IOException("request timed out"); + } + try { + Thread.sleep(5); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + throw new IOException("interrupted while reading request"); } - out.write(b); } - buffer.clear(); } - return out.size() == 0 ? null : out.toString(StandardCharsets.UTF_8); } private static Path socketPath() { @@ -136,11 +165,19 @@ public final class Daemon { return dir.resolve("daemon.sock"); } - private static void trySecureDir(Path dir) { + // Make the socket dir private, and refuse to use it if it is not ours. The /tmp fallback + // path is predictable, so another user could have pre-created it (the classic /tmp race); + // binding a socket inside a directory someone else owns would hand them control of it. + private static void secureDir(Path dir) throws IOException { try { Files.setPosixFilePermissions(dir, PosixFilePermissions.fromString("rwx------")); - } catch (IOException | UnsupportedOperationException ignored) { - // Best effort: XDG_RUNTIME_DIR is already user-private; the /tmp fallback we try to lock. + } catch (UnsupportedOperationException ignored) { + return; // not a POSIX filesystem: nothing more we can check + } + String owner = Files.getOwner(dir, LinkOption.NOFOLLOW_LINKS).getName(); + String user = System.getProperty("user.name"); + if (!owner.equals(user)) { + throw new IOException(dir + " is owned by '" + owner + "', not '" + user + "'"); } } } diff --git a/src/main/java/com/gregor/jprototerm/GlyphCache.java b/src/main/java/com/gregor/jprototerm/GlyphCache.java index 187b159..5488770 100644 --- a/src/main/java/com/gregor/jprototerm/GlyphCache.java +++ b/src/main/java/com/gregor/jprototerm/GlyphCache.java @@ -29,6 +29,11 @@ final class GlyphCache { record Glyph(int width, int height, byte[] alpha) { } + // Bounds the atlas so pathological glyph diversity (e.g. a dump of distinct CJK/emoji cells) + // can't grow it without limit; on overflow it clears and rebuilds on demand, like any + // metrics change. + private static final int MAX_GLYPHS = 4096; + private final TerminalMetrics metrics; private final Map glyphs = new HashMap<>(); // The metrics snapshot the cached glyphs were rasterized for; a mismatch clears the cache. @@ -43,7 +48,16 @@ final class GlyphCache { Glyph glyph(String text) { ensureCurrent(); - return glyphs.computeIfAbsent(text, this::renderGlyph); + Glyph cached = glyphs.get(text); + if (cached != null) { + return cached; + } + if (glyphs.size() >= MAX_GLYPHS) { + glyphs.clear(); + } + Glyph rendered = renderGlyph(text); + glyphs.put(text, rendered); + return rendered; } // Drop the rasterized masks if the font/cell geometry changed since they were built. Cheap to diff --git a/src/main/java/com/gregor/jprototerm/KeyBinding.java b/src/main/java/com/gregor/jprototerm/KeyBinding.java index f7c5cab..57b1250 100644 --- a/src/main/java/com/gregor/jprototerm/KeyBinding.java +++ b/src/main/java/com/gregor/jprototerm/KeyBinding.java @@ -5,17 +5,19 @@ import javafx.scene.input.KeyEvent; import java.util.Locale; -public record KeyBinding(boolean alt, boolean control, boolean shift, KeyCode code) { +public record KeyBinding(boolean alt, boolean control, boolean shift, boolean meta, KeyCode code) { public static KeyBinding parse(String value) { boolean alt = false; boolean control = false; boolean shift = false; + boolean meta = false; KeyCode code = null; for (String part : value.split("\\+")) { String token = part.trim().toUpperCase(Locale.ROOT); switch (token) { - case "ALT", "META" -> alt = true; + case "ALT" -> alt = true; + case "META", "SUPER" -> meta = true; case "CTRL", "CONTROL" -> control = true; case "SHIFT" -> shift = true; default -> code = keyCode(token); @@ -25,13 +27,14 @@ public record KeyBinding(boolean alt, boolean control, boolean shift, KeyCode co if (code == null) { throw new IllegalArgumentException("Key binding has no key code: " + value); } - return new KeyBinding(alt, control, shift, code); + return new KeyBinding(alt, control, shift, meta, code); } public boolean matches(KeyEvent event) { return event.isAltDown() == alt && event.isControlDown() == control && event.isShiftDown() == shift + && event.isMetaDown() == meta && event.getCode() == code; } @@ -44,6 +47,9 @@ public record KeyBinding(boolean alt, boolean control, boolean shift, KeyCode co if (alt) { builder.append("ALT+"); } + if (meta) { + builder.append("META+"); + } if (shift) { builder.append("SHIFT+"); } diff --git a/src/main/java/com/gregor/jprototerm/LinuxPty.java b/src/main/java/com/gregor/jprototerm/LinuxPty.java index 4427f4c..325e1cf 100644 --- a/src/main/java/com/gregor/jprototerm/LinuxPty.java +++ b/src/main/java/com/gregor/jprototerm/LinuxPty.java @@ -317,6 +317,10 @@ public final class LinuxPty implements AutoCloseable { } private void closeMaster() { + // Note: closing the master fd does NOT wake a reader thread blocked in read() on it — + // the reader unblocks via EOF when the child exits and the slave end closes. The signal + // here usually does that; if the child ignores it, the SIGKILL escalation in reap() + // guarantees it shortly after. callKill(pid, closeSignal); callInt(CLOSE, masterFd); } diff --git a/src/main/java/com/gregor/jprototerm/StartupTiming.java b/src/main/java/com/gregor/jprototerm/StartupTiming.java index d5fe286..cbf4d82 100644 --- a/src/main/java/com/gregor/jprototerm/StartupTiming.java +++ b/src/main/java/com/gregor/jprototerm/StartupTiming.java @@ -22,8 +22,11 @@ final class StartupTiming { private StartupTiming() { } - /** Records a phase boundary, printing the delta since the previous mark and since JVM start. */ - static void mark(String phase) { + /** + * Records a phase boundary, printing the delta since the previous mark and since JVM start. + * Synchronized because marks come from both the launcher thread and the FX thread. + */ + static synchronized void mark(String phase) { if (!ENABLED) { return; } @@ -38,7 +41,7 @@ final class StartupTiming { * Records the first rendered frame exactly once, then becomes a no-op. Safe and cheap to call * from the render loop every frame (it only ever touches FX-thread state). */ - static void firstFrame() { + static synchronized void firstFrame() { if (!ENABLED || firstFrameSeen) { return; } diff --git a/src/main/java/com/gregor/jprototerm/Tab.java b/src/main/java/com/gregor/jprototerm/Tab.java index b854952..3824d6e 100644 --- a/src/main/java/com/gregor/jprototerm/Tab.java +++ b/src/main/java/com/gregor/jprototerm/Tab.java @@ -18,6 +18,14 @@ import java.util.stream.Stream; * return whether they actually changed anything so it can bump its layout version. */ final class Tab implements AutoCloseable { + // Floating-pane sizing policy: a fraction of the tab's size with a floor so panes stay + // usable in small windows, cascaded diagonally per pane, kept off the window edge. + private static final double FLOATING_SIZE_FRACTION = 0.58; + private static final double FLOATING_MIN_WIDTH = 420.0; + private static final double FLOATING_MIN_HEIGHT = 260.0; + private static final double FLOATING_CASCADE_OFFSET = 28.0; + private static final double FLOATING_EDGE_MARGIN = 12.0; + private final AppConfig config; private final TerminalMetrics metrics; // Notified (on the FX thread) when one of this tab's panes' process exits on its own, so the @@ -142,13 +150,15 @@ final class Tab implements AutoCloseable { tiled.get(i).bounds(i * tileWidth, topInset, tileWidth, availHeight); } - double floatingWidth = Math.max(420, width * 0.58); - double floatingHeight = Math.max(260, availHeight * 0.58); + double floatingWidth = Math.max(FLOATING_MIN_WIDTH, width * FLOATING_SIZE_FRACTION); + double floatingHeight = Math.max(FLOATING_MIN_HEIGHT, availHeight * FLOATING_SIZE_FRACTION); for (int i = 0; i < floating.size(); i++) { - double offset = i * 28.0; + double offset = i * FLOATING_CASCADE_OFFSET; floating.get(i).bounds( - Math.min(width - floatingWidth - 12.0, ((width - floatingWidth) / 2.0) + offset), - Math.min(height - floatingHeight - 12.0, topInset + ((availHeight - floatingHeight) / 2.0) + offset), + Math.min(width - floatingWidth - FLOATING_EDGE_MARGIN, + ((width - floatingWidth) / 2.0) + offset), + Math.min(height - floatingHeight - FLOATING_EDGE_MARGIN, + topInset + ((availHeight - floatingHeight) / 2.0) + offset), floatingWidth, floatingHeight); } @@ -384,7 +394,9 @@ final class Tab implements AutoCloseable { private double[] paneSize(boolean asFloating) { double availHeight = lastHeight - lastTopInset; if (asFloating) { - return new double[] {Math.max(420, lastWidth * 0.58), Math.max(260, availHeight * 0.58)}; + return new double[] { + Math.max(FLOATING_MIN_WIDTH, lastWidth * FLOATING_SIZE_FRACTION), + Math.max(FLOATING_MIN_HEIGHT, availHeight * FLOATING_SIZE_FRACTION)}; } // A new tiled pane joins the row, so each gets 1/(n+1) of the width. return new double[] {lastWidth / (tiled.size() + 1), availHeight}; diff --git a/src/main/java/com/gregor/jprototerm/TerminalPane.java b/src/main/java/com/gregor/jprototerm/TerminalPane.java index be67215..32ebbd6 100644 --- a/src/main/java/com/gregor/jprototerm/TerminalPane.java +++ b/src/main/java/com/gregor/jprototerm/TerminalPane.java @@ -18,6 +18,7 @@ import javafx.scene.shape.Shape; import java.util.Optional; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Supplier; /** * One terminal: owns its ghostty {@link Terminal}, the {@link ShellSession}/pty driving it, @@ -80,8 +81,8 @@ public final class TerminalPane implements AutoCloseable, RenderTarget { public static TerminalPane create(AppConfig config, TerminalMetrics metrics, Runnable onContentChange, double widthPx, double heightPx, String workingDirectory) { TerminalPane pane = newPane(config, metrics, onContentChange, widthPx, heightPx); - pane.attach(ShellSession.start(config.shell(), config.envOverride(), pane, pane.columns, pane.rows, - workingDirectory, config.closeSignalNumber())); + attachOrShowError(pane, () -> ShellSession.start(config.shell(), config.envOverride(), pane, + pane.columns, pane.rows, workingDirectory, config.closeSignalNumber())); return pane; } @@ -93,11 +94,23 @@ public final class TerminalPane implements AutoCloseable, RenderTarget { public static TerminalPane createWithCommand(AppConfig config, TerminalMetrics metrics, Runnable onContentChange, double widthPx, double heightPx, String workingDirectory, String command) { TerminalPane pane = newPane(config, metrics, onContentChange, widthPx, heightPx); - pane.attach(ShellSession.startCommand(config.envOverride(), pane, pane.columns, pane.rows, - workingDirectory, command, config.closeSignalNumber())); + attachOrShowError(pane, () -> ShellSession.startCommand(config.envOverride(), pane, + pane.columns, pane.rows, workingDirectory, command, config.closeSignalNumber())); return pane; } + // Start the pane's process, but never let a spawn failure (e.g. a bad `shell` in config) + // propagate and crash window/pane creation. ShellSession has already written the error into + // the pane, so the pane opens showing it; with no session attached it is inert (sends are + // dropped) and the user closes it with the close-pane key. + private static void attachOrShowError(TerminalPane pane, Supplier start) { + try { + pane.attach(start.get()); + } catch (RuntimeException ex) { + System.err.println("jprototerm: " + ex.getMessage()); + } + } + private static TerminalPane newPane(AppConfig config, TerminalMetrics metrics, Runnable onContentChange, double widthPx, double heightPx) { int columns = widthPx > 0 ? metrics.columnsFor(widthPx) : config.columns(); diff --git a/src/main/java/com/gregor/jprototerm/TerminalWindow.java b/src/main/java/com/gregor/jprototerm/TerminalWindow.java index d69b4bb..4c38748 100644 --- a/src/main/java/com/gregor/jprototerm/TerminalWindow.java +++ b/src/main/java/com/gregor/jprototerm/TerminalWindow.java @@ -138,8 +138,7 @@ final class TerminalWindow { } String encoded = KeyEncoder.encode(event); if (encoded != null) { - compositor.activePane().send(encoded); - event.consume(); + sendToActivePane(encoded, event); } } @@ -150,15 +149,25 @@ final class TerminalWindow { String text = event.getCharacter(); if (text != null && !text.isEmpty() && text.charAt(0) >= 0x20 && text.charAt(0) != 0x7f) { - compositor.activePane().send(text); + sendToActivePane(text, event); + } + } + + // Key handlers run on every keystroke, including any that race the window's teardown, so + // tolerate the no-pane-left state instead of assuming one exists. + private void sendToActivePane(String text, KeyEvent event) { + TerminalPane active = compositor.activePane(); + if (active != null) { + active.send(text); event.consume(); } } private void pasteFromClipboard() { + TerminalPane active = compositor.activePane(); Clipboard clipboard = Clipboard.getSystemClipboard(); - if (clipboard.hasString()) { - compositor.activePane().paste(clipboard.getString()); + if (active != null && clipboard.hasString()) { + active.paste(clipboard.getString()); } } @@ -206,17 +215,22 @@ final class TerminalWindow { } private void openScrollbackInEditor() { + // Capture the active pane's scrollback before opening the floating pane, since that + // makes the new pane active. + TerminalPane active = compositor.activePane(); + if (active == null) { + return; + } try { - // Capture the active pane's scrollback before opening the floating pane, since that - // makes the new pane active. Path file = Files.createTempFile("jprototerm-scrollback-", ".txt"); - Files.writeString(file, compositor.activePane().scrollbackText()); - file.toFile().deleteOnExit(); + Files.writeString(file, active.scrollbackText()); // Run the editor as the floating pane's process (via /bin/sh -c) rather than typing the // command into an interactive shell. The command runs deterministically from the start - // — no shell startup/rc race — and the pane auto-closes when the editor exits. - compositor.openFloatingPane(scrollbackEditorCommand(file)); + // — no shell startup/rc race — and the pane auto-closes when the editor exits. The + // trailing rm removes the file (which holds terminal contents) when the editor exits; + // deleteOnExit would leak files for the JVM's whole lifetime in daemon mode. + compositor.openFloatingPane(scrollbackEditorCommand(file) + "; rm -f " + shellQuote(file.toString())); } catch (IOException ex) { System.err.println("Could not open scrollback in editor: " + ex.getMessage()); }