diff --git a/src/apprt/gtk/portal/OpenURI.zig b/src/apprt/gtk/portal/OpenURI.zig index 985583741..97aa013e5 100644 --- a/src/apprt/gtk/portal/OpenURI.zig +++ b/src/apprt/gtk/portal/OpenURI.zig @@ -31,6 +31,50 @@ entries: std.AutoArrayHashMapUnmanaged(usize, *Entry) = .empty, /// Used to manage a timer to clean up any orphan entries in the map. cleanup_timer: ?c_uint = null, +/// Set to false during shutdown so callbacks stop touching internal state. +alive: bool = true, + +const RequestData = struct { + open_uri: *OpenURI, + token: usize, + kind: apprt.action.OpenUrl.Kind, + uri: [:0]const u8, + request_path: [:0]const u8, + + pub fn init( + alloc: Allocator, + open_uri: *OpenURI, + token: usize, + kind: apprt.action.OpenUrl.Kind, + uri: []const u8, + request_path: []const u8, + ) Allocator.Error!*RequestData { + const uri_copy = try alloc.dupeZ(u8, uri); + errdefer alloc.free(uri_copy); + + const request_path_copy = try alloc.dupeZ(u8, request_path); + errdefer alloc.free(request_path_copy); + + const data = try alloc.create(RequestData); + errdefer alloc.destroy(data); + + data.* = .{ + .open_uri = open_uri, + .token = token, + .kind = kind, + .uri = uri_copy, + .request_path = request_path_copy, + }; + + return data; + } + + pub fn deinit(self: *const RequestData, alloc: Allocator) void { + alloc.free(self.uri); + alloc.free(self.request_path); + } +}; + /// Data about any in-flight calls to the portal. pub const Entry = struct { /// When the request started. @@ -47,7 +91,7 @@ pub const Entry = struct { /// method calls are asynchronous and the original may have been freed by /// the time we need it. uri: [:0]const u8, - /// Used to manage a scription to a D-Bus signal, which is how the XDG + /// Used to manage a subscription to a D-Bus signal, which is how the XDG /// Portal reports results of the method call. subscription: ?c_uint = null, @@ -82,23 +126,36 @@ pub fn deinit(self: *OpenURI) void { self.mutex.lock(); defer self.mutex.unlock(); + if (!self.alive) return; + self.alive = false; + self.stopCleanupTimer(); for (self.entries.entries.items(.value)) |entry| { - entry.deinit(alloc); - alloc.destroy(entry); + self.unsubscribeFromResponse(entry); + destroyEntry(alloc, entry); } self.entries.deinit(alloc); + self.entries = .empty; + self.dbus = null; } /// Send the D-Bus method call to the XDG Desktop portal. The result of the /// method call will be reported asynchronously. -pub fn start(self: *OpenURI, value: apprt.action.OpenUrl) (Allocator.Error || std.fmt.BufPrintError || Errors)!void { +pub fn start(self: *OpenURI, value: apprt.action.OpenUrl) (Allocator.Error || Errors)!void { const alloc = self.app.app.allocator(); const dbus = self.dbus orelse return error.DBusConnectionRequired; const token = portal.generateToken(); + const request_path = try portal.getRequestPath(alloc, dbus, token); + defer alloc.free(request_path); + + const request = try RequestData.init(alloc, self, token, value.kind, value.url, request_path); + errdefer { + request.deinit(alloc); + alloc.destroy(request); + } self.mutex.lock(); defer self.mutex.unlock(); @@ -121,16 +178,12 @@ pub fn start(self: *OpenURI, value: apprt.action.OpenUrl) (Allocator.Error || st errdefer { _ = self.entries.swapRemove(token); - entry.deinit(alloc); - alloc.destroy(entry); + destroyEntry(alloc, entry); } self.startCleanupTimer(); - - try self.subscribeToResponse(entry, dbus); - errdefer self.unsubscribeFromResponse(entry); - - try self.sendRequest(entry, dbus); + self.subscribeToResponse(entry, dbus, request_path.ptr); + self.sendRequest(entry, dbus, request); } /// Subscribe to the D-Bus signal that will contain the results of our method @@ -139,16 +192,12 @@ fn subscribeToResponse( self: *OpenURI, entry: *Entry, dbus: *gio.DBusConnection, -) (Allocator.Error || std.fmt.BufPrintError || Errors)!void { + request_path: [*:0]const u8, +) void { assert(!self.mutex.tryLock()); - const alloc = self.app.app.allocator(); - if (entry.subscription != null) return; - const request_path = try portal.getRequestPath(alloc, dbus, entry.token); - defer alloc.free(request_path); - entry.subscription = dbus.signalSubscribe( null, "org.freedesktop.portal.Request", @@ -180,17 +229,40 @@ fn unsubscribeFromResponse(self: *OpenURI, entry: *Entry) void { } } +fn destroyEntry(alloc: Allocator, entry: *Entry) void { + entry.deinit(alloc); + alloc.destroy(entry); +} + +fn failRequest(self: *OpenURI, token: usize) ?*Entry { + self.mutex.lock(); + defer self.mutex.unlock(); + + if (!self.alive) return null; + + const entry = (self.entries.fetchSwapRemove(token) orelse return null).value; + self.unsubscribeFromResponse(entry); + return entry; +} + +fn failRequestAndFallback(self: *OpenURI, request: *const RequestData) void { + const alloc = self.app.app.allocator(); + const entry = self.failRequest(request.token) orelse return; + defer destroyEntry(alloc, entry); + + self.app.app.openUrlFallback(request.kind, request.uri); +} + /// Send the D-Bus method call to the portal. The mutex must be locked when this /// is called. fn sendRequest( self: *OpenURI, entry: *Entry, dbus: *gio.DBusConnection, -) Allocator.Error!void { + request: *RequestData, +) void { assert(!self.mutex.tryLock()); - const alloc = self.app.app.allocator(); - const payload = payload: { const builder_type = glib.VariantType.new("(ssa{sv})"); defer builder_type.free(); @@ -198,7 +270,6 @@ fn sendRequest( // Initialize our builder to build up our parameters var builder: glib.VariantBuilder = undefined; builder.init(builder_type); - errdefer builder.clear(); // parent window - empty string means we have no window builder.add("s", ""); @@ -223,8 +294,8 @@ fn sendRequest( builder.add("s", "handle_token"); - const token = try std.fmt.allocPrintSentinel(alloc, "{x:0<16}", .{entry.token}, 0); - defer alloc.free(token); + var token_buf: portal.TokenBuffer = undefined; + const token = portal.formatToken(&token_buf, entry.token); const handle_token = glib.Variant.newString(token.ptr); builder.add("v", handle_token); @@ -273,7 +344,7 @@ fn sendRequest( -1, null, requestCallback, - self, + request, ); } @@ -281,13 +352,25 @@ fn sendRequest( /// not indicate that the that the method call succeeded but it may contain an /// error message that is useful to log for debugging purposes. fn requestCallback( - _: ?*gobject.Object, + source: ?*gobject.Object, result: *gio.AsyncResult, ud: ?*anyopaque, ) callconv(.c) void { - const self: *OpenURI = @ptrCast(@alignCast(ud orelse return)); - const dbus = self.dbus orelse { - log.err("Open URI request finished without a D-Bus connection", .{}); + const request: *RequestData = @ptrCast(@alignCast(ud orelse return)); + const self = request.open_uri; + const alloc = self.app.app.allocator(); + defer { + request.deinit(alloc); + alloc.destroy(request); + } + + const dbus = gobject.ext.cast(gio.DBusConnection, source orelse { + log.err("Open URI request finished without a D-Bus source object", .{}); + self.failRequestAndFallback(request); + return; + }) orelse { + log.err("Open URI request finished with an unexpected source object", .{}); + self.failRequestAndFallback(request); return; }; @@ -301,11 +384,13 @@ fn requestCallback( err.f_message orelse "(unknown)", err.f_code, }); + self.failRequestAndFallback(request); return; } const reply = reply_ orelse { log.err("D-Bus method call returned a null value!", .{}); + self.failRequestAndFallback(request); return; }; defer reply.unref(); @@ -315,29 +400,42 @@ fn requestCallback( if (reply.isOfType(reply_type) == 0) { log.warn("Reply from D-Bus method call does not contain an object path!", .{}); + self.failRequestAndFallback(request); return; } - var object_path_: ?[*:0]const u8 = null; - reply.get("(o)", &object_path_); + var object_path: [*:0]const u8 = undefined; + reply.get("(&o)", &object_path); - const object_path = object_path_ orelse { - log.err("D-Bus method call did not return an object path", .{}); - return; - }; - - const token = portal.parseRequestPath(std.mem.span(object_path)) orelse { + const token = portal.parseRequestPathToken(std.mem.span(object_path)) orelse { log.warn("Unable to parse token from the object path {s}", .{object_path}); + self.failRequestAndFallback(request); return; }; - // Check to see if the request path returned matches a token that we sent. + if (token != request.token) { + log.warn("Open URI request returned mismatched token expected={x} actual={x}", .{ + request.token, + token, + }); + self.failRequestAndFallback(request); + return; + } + self.mutex.lock(); defer self.mutex.unlock(); - if (!self.entries.contains(token)) { - log.warn("Token {x:0<16} not found in the map!", .{token}); - } + if (!self.alive) return; + + const entry = self.entries.get(token) orelse return; + if (std.mem.eql(u8, request.request_path, std.mem.span(object_path))) return; + + log.debug("updating open uri request path old={s} new={s}", .{ + request.request_path, + object_path, + }); + self.unsubscribeFromResponse(entry); + self.subscribeToResponse(entry, dbus, object_path); } /// Handle the response signal from the portal. This should contain the actual @@ -358,7 +456,7 @@ fn responseReceived( const alloc = self.app.app.allocator(); - const token = portal.parseRequestPath(std.mem.span(object_path)) orelse { + const token = portal.parseRequestPathToken(std.mem.span(object_path)) orelse { log.warn("invalid object path: {s}", .{std.mem.span(object_path)}); return; }; @@ -366,20 +464,20 @@ fn responseReceived( self.mutex.lock(); defer self.mutex.unlock(); + if (!self.alive) return; + const entry = (self.entries.fetchSwapRemove(token) orelse { - log.warn("no entry for token {x:0<16}", .{token}); + log.warn("no entry for token {x}", .{token}); return; }).value; - defer { - entry.deinit(alloc); - alloc.destroy(entry); - } + defer destroyEntry(alloc, entry); self.unsubscribeFromResponse(entry); var response: u32 = 0; var results: ?*glib.Variant = null; + defer if (results) |variant| variant.unref(); params.get("(u@a{sv})", &response, &results); switch (response) { @@ -440,6 +538,7 @@ fn cleanup(ud: ?*anyopaque) callconv(.c) c_int { defer self.mutex.unlock(); self.cleanup_timer = null; + if (!self.alive) return @intFromBool(glib.SOURCE_REMOVE); const now = std.time.Instant.now() catch { // `now()` should never fail, but if it does, don't crash, just return. @@ -451,10 +550,11 @@ fn cleanup(ud: ?*anyopaque) callconv(.c) c_int { loop: while (true) { for (self.entries.entries.items(.value)) |entry| { if (now.since(entry.start) > cleanup_timeout * std.time.ns_per_s) { + log.warn("open uri request timed out token={x}", .{entry.token}); self.unsubscribeFromResponse(entry); _ = self.entries.swapRemove(entry.token); - entry.deinit(alloc); - alloc.destroy(entry); + self.app.app.openUrlFallback(entry.kind, entry.uri); + destroyEntry(alloc, entry); continue :loop; } }