Crash in os.flatpak.FlatpakHostCommand.start when constructing the RPC parameters using g_variant_new()

[yorickpeterse]

When building with -Dflatpak=true and running the resulting executable in a Flatpak, Ghostty crashes here:

ghostty/src/os/flatpak.zig

Lines 286 to 293 in bee2188

	const params = c.g_variant_new(
	"(^ay^aay@a{uh}@a{ss}u)",
	@as(*const anyopaque, if (self.cwd) |*cwd| cwd.ptr else g_cwd),
	args,
	c.g_variant_builder_end(fd_builder),
	c.g_variant_builder_end(env_builder),
	@as(c_int, 0),
	);

The GDB trace of the thread in question at this stage is as follows:

#0  0x00007ffff6d3a2dd in __strlen_avx2 () at /usr/lib/x86_64-linux-gnu/libc.so.6
#1  0x00007ffff715c218 in g_variant_new_bytestring () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007ffff715c2ba in g_variant_new_bytestring_array () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007ffff7162383 in g_variant_valist_new () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#4  0x00007ffff7162970 in g_variant_new_va () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#5  0x00007ffff7162aa6 in g_variant_new () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#6  0x0000555557ca1a25 in os.flatpak.FlatpakHostCommand.start (self=0x7fffffff3c90, alloc=..., bus=0x7fffec00c900, loop=0x7fffec016b80)
    at /run/build/ghostty/ghostty/src/os/flatpak.zig:286
#7  0x0000555557c18bae in os.flatpak.FlatpakHostCommand.threadMain (self=0x7fffffff3c90, alloc=...) at /run/build/ghostty/ghostty/src/os/flatpak.zig:206
#8  0x0000555557b8f7f2 in Thread.callFn__anon_130282 (args=...) at /usr/lib/sdk/ziglang/lib/std/Thread.zig:408
#9  0x0000555557a70263 in Thread.PosixThreadImpl.spawn__anon_118977.Instance.entryFn (raw_arg=0x555559a459b0)
    at /usr/lib/sdk/ziglang/lib/std/Thread.zig:674
#10 0x00007ffff6c5ae0e in start_thread () at /usr/lib/x86_64-linux-gnu/libc.so.6
#11 0x00007ffff6cdf7d4 in clone () at /usr/lib/x86_64-linux-gnu/libc.so.6

The details of the locals/arguments in frame 6 are as follows:

(gdb) info locals
err = 0x0
arena_allocator = {child_allocator = {ptr = 0x7fffffff4910, vtable = 0x555559959730 <__anon_153184>}, state = {buffer_list = {first = 0x7ffff5600b88},
    end_index = 42}}
arena = {ptr = 0x7ffff11fdce8, vtable = 0x555559959730 <__anon_153184>}
fd_list = 0x7fffec015f20
fd_builder = 0x7fffec0125d0
env_builder = 0x7fffec015f50
args_ptr = 0x7fffec01
args = 0x7fffec013280
g_cwd = 0x7fffec015d4
params = 0x7fffd40028c0
subscription_id = 32767
reply = 0x7fffec00c920
pid = 0
(gdb) info args
self = 0x7fffffff3c90
alloc = <optimized out>
bus = 0x7fffec00c900
loop = 0x7fffec016b80

I'm not entirely sure what the code in question does and more precisely what it should do instead. To figure this out, somebody with better knowledge of Glib would have to help out, as I know very little of it.

Reading material

• The DBus command these parameters are used for (I think?): https://docs.flatpak.org/en/latest/libflatpak-api-reference.html#gdbus-method-org-freedesktop-Flatpak-Development.HostCommand

[yorickpeterse]

To test this (from the top of my head):

1. git clone https://github.com/yorickpeterse/ghostty-flatpak.git
2. cd ghostty-flatpak
3. bash deps.sh
4. Apply the following patch:

diff --git a/com.mitchellh.ghostty.yml b/com.mitchellh.ghostty.yml
index cb8e295..0521103 100644
--- a/com.mitchellh.ghostty.yml
+++ b/com.mitchellh.ghostty.yml
@@ -6,6 +6,8 @@ sdk: org.gnome.Sdk
 sdk-extensions:
   - org.freedesktop.Sdk.Extension.ziglang
 command: ghostty
+build-options:
+  no-debuginfo: true
 finish-args:
   - "--allow=devel"
   - "--device=dri"
@@ -21,13 +23,13 @@ modules:
     build-options:
       append-path: /usr/lib/sdk/ziglang
     build-commands:
-      - cd ghostty && patch -p1 --forward < ../flatpak.patch
+      # - cd ghostty && patch -p1 --forward < ../flatpak.patch
       - tar -xf vendor.tar.gz -C ghostty
       # The listed integrations are _disabled_ such that we _do_ build them from
       # source. This way we don't have to add each library as an explicit
       # dependency, simplifying the build process.
       - cd ghostty && zig build
-        -Doptimize=ReleaseFast
+        -Doptimize=Debug
         -Dgtk-adwaita=true
         -Dcpu=baseline
         -Dflatpak=true
@@ -43,9 +45,12 @@ modules:
         --prefix /app
       - install -Dm644 ${FLATPAK_ID}.metainfo.xml -t /app/share/metainfo/
     sources:
-      - type: archive
-        url: https://release.files.ghostty.org/1.0.0/ghostty-source.tar.gz
-        sha256: 7fcb5fed08bd23d54be138af4f63a78cf5addddbe40322465b520cf14c46f181
+      # - type: archive
+      #   url: https://release.files.ghostty.org/1.0.0/ghostty-source.tar.gz
+      #   sha256: 7fcb5fed08bd23d54be138af4f63a78cf5addddbe40322465b520cf14c46f181
+      #   dest: ghostty
+      - type: dir
+        path: /var/home/yorickpeterse/Projects/zig/ghostty
         dest: ghostty
       - type: dir
         path: data

Then open com.mitchellh.ghostty.yml and replace /var/home/yorickpeterse/Projects/zig/ghostty with the path to your local Ghostty installation so you can make changes where necessary. Then to build it:

flatpak run org.flatpak.Builder --force-clean --install-deps-from=flathub --ccache --mirror-screenshots-url=https://dl.flathub.org/media/ --repo=repo builddir com.mitchellh.ghostty.yml
flatpak install ./repo com.mitchellh.ghostty --reinstall

Then to run a debug session:

flatpak run --devel --command=bash com.mitchellh.ghostty
gdb /app/bin/ghostty

Then press r to start the session, followed by the usual GDB commands to print backtraces and what not.

[chergert]

👋 Ptyxis author here just peeking in.

#0  0x00007ffff6d3a2dd in __strlen_avx2 () at /usr/lib/x86_64-linux-gnu/libc.so.6

Crashing in strlen() is almost always because it was passed NULL.

If you're going the route of calling the D-Bus API directly, here is an example of doing that from my IDE project GNOME Builder, https://gitlab.gnome.org/GNOME/gnome-builder/-/blob/main/src/libide/threading/ide-flatpak-subprocess.c#L1385 which should be pretty easy to rewrite in Zig.

[yorickpeterse]

Forgot to add this over the weekend: I looked at the data passed to the glib function in question, but it seems everything is valid, i.e. there are no NULL pointers passed for the PWD or the arguments as far as I was able to tell. It's possible fd_builder and/or env_builder are in an invalid state, but I didn't check that at the time.

If anybody is reading this and has a familiarity with both Zig and the glib API, help with this would be greatly appreciated 😃

[chergert]

The next thing I would look into then is the ownership transfer semantics of GLib bindings in Zig. Newly created GVariant are floating in nature, meaning that when you use them in the GDBus calls, they will sink that floating reference (e.g. steal it).

[yorickpeterse]

@mitchellh Seeing as this is an actual bug, would it be possible to promote this discussion into an issue? That might also help with the visibility 😃

[alaviss]

I opened a PR (#5213) with the fix for this, but I still haven't got the actual terminal working yet. It stopped crashing at least, though.

[yorickpeterse]

@alaviss Thanks! That's at least a step in the right direction.

When you say it doesn't work yet, do you mean it still doesn't spawn a shell correctly?

[yorickpeterse]

Ah, I see what happens now: it doesn't crash, but instead it seems to just hang with a blank screen of sorts:

[yorickpeterse]

Curiously, when opening a new tab I see the following:

[alaviss]

So I narrowed it down to this:

ghostty/src/termio/Exec.zig

Lines 98 to 102 in 72d0855

	// Get the pid from the subprocess
	const pid = pid: {
	const command = self.subprocess.command orelse return error.ProcessNotStarted;
	break :pid command.pid orelse return error.ProcessNoPid;
	};

The flatpak process start code did not populate the .command struct, so ghostty thought an error occurred and terminated the shell immediately.

From what I can tell, this can be fixed by obtaining the pid from .flatpak_command, but it might not mesh well with process exit detection code in there (since flatpak shell is not a direct children of ghostty).