deal with large outputs from xdg-open/rundll32/open (#12797)

Depending on your system config, `xdg-open` may stay open for extended
periods, and potentially log more than the 50kb of output that we were
previously able to deal with. This changes `open()` so that output on
`stdout` is just directly ignored. Any output from `stderr` is immedialy
logged rather than collected for later logging.

Note that this will generally occur if your system is not configured
with the DBus portals that `xdg-open` uses to open URLs rather than
launching programs like your web browser directly. This could be seen as
user misconfiguration but we should deal with it robustly anyway.

src/os/open.zig

@@ -9,13 +9,11 @@ const log = std.log.scoped(.@"os-open");
 /// Open a URL in the default handling application.
 ///
 /// Any output on stderr is logged as a warning in the application logs.
-/// Output on stdout is ignored. The allocator is used to buffer the
-/// log output and may allocate from another thread.
+/// Output on stdout is ignored.
 ///
-/// This function is purposely simple for the sake of providing
-/// some portable way to open URLs. If you are implementing an
-/// apprt for Ghostty, you should consider doing something special-cased
-/// for your platform.
+/// This function is purposely simple for the sake of providing some portable
+/// way to open URLs. If you are implementing an apprt for Ghostty, you should
+/// consider doing something special-cased for your platform.
 pub fn open(
     alloc: Allocator,
     kind: apprt.action.OpenUrl.Kind,
@@ -44,14 +42,16 @@ pub fn open(
         else => @compileError("unsupported OS"),
     };
 
-    // Pipe stdout/stderr so we can collect output from the command.
-    // This must be set before spawning the process.
-    exe.stdout_behavior = .Pipe;
+    // Ignore anything from stdout. This must be set before spawning the
+    // process.
+    exe.stdout_behavior = .Ignore;
+    // Pipe stderr so we can log the stderr from the command. This must be set
+    // before spawning the process.
     exe.stderr_behavior = .Pipe;
 
     // In the snap on Linux the launcher exports LD_LIBRARY_PATH pointing at
-    // the snap's bundled libraries. Leaking this into child process can
-    // can be problematic, so let's drop it from the env
+    // the snap's bundled libraries. Leaking this into child process can can be
+    // problematic, so let's drop it from the env
     var snap_env: std.process.EnvMap = if (comptime build_config.snap) blk: {
         var env = try std.process.getEnvMap(alloc);
         env.remove("LD_LIBRARY_PATH");
@@ -64,34 +64,34 @@ pub fn open(
     // quickly.
     try exe.spawn();
 
-    // Create a thread that handles collecting output and reaping
-    // the process. This is done in a separate thread because SOME
-    // open implementations block and some do not. It's easier to just
-    // spawn a thread to handle this so that we never block.
-    const thread = try std.Thread.spawn(.{}, openThread, .{ alloc, exe });
+    // Create a thread that handles collecting output and reaping the process.
+    // This is done in a separate thread because SOME open implementations block
+    // and some do not. It's easier to just spawn a thread to handle this so
+    // that we never block.
+    const thread = try std.Thread.spawn(.{}, openThread, .{exe});
     thread.detach();
 }
 
-fn openThread(alloc: Allocator, exe_: std.process.Child) !void {
-    // 50 KiB is the default value used by std.process.Child.run and should
-    // be enough to get the output we care about.
-    const output_max_size = 50 * 1024;
-
-    var stdout: std.ArrayListUnmanaged(u8) = .{};
-    var stderr: std.ArrayListUnmanaged(u8) = .{};
-    defer {
-        stdout.deinit(alloc);
-        stderr.deinit(alloc);
-    }
-
+fn openThread(exe_: std.process.Child) void {
     // Copy the exe so it is non-const. This is necessary because wait()
     // requires a mutable reference and we can't have one as a thread
     // param.
     var exe = exe_;
-    try exe.collectOutput(alloc, &stdout, &stderr, output_max_size);
-    _ = try exe.wait();
-
-    // If we have any stderr output we log it. This makes it easier for
-    // users to debug why some open commands may not work as expected.
-    if (stderr.items.len > 0) log.warn("wait stderr={s}", .{stderr.items});
+    if (exe.stderr) |stderr| {
+        var buffer: [256]u8 = undefined;
+        var stream = stderr.readerStreaming(&buffer);
+        const reader = &stream.interface;
+        while (true) {
+            const line = reader.takeDelimiterExclusive('\n') catch |outer| switch (outer) {
+                error.EndOfStream => break,
+                error.ReadFailed => break,
+                error.StreamTooLong => reader.take(buffer.len) catch |inner| switch (inner) {
+                    error.ReadFailed => break,
+                    error.EndOfStream => break,
+                },
+            };
+            log.warn("open stderr={s}", .{line});
+        }
+    }
+    _ = exe.wait() catch {};
 }