deal with large outputs from xdg-open/rundll32/open
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.pull/12797/head
parent
c5946f4fef
commit
bb375a2f75
|
|
@ -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 {};
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue