mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-05-28 15:55:20 +00:00
apprt/gtk: reuse one audio-bell MediaFile per surface to fix thread leak
Each audio bell called gtk.MediaFile.newForFilename, which spins up a full GStreamer pipeline. The GTK4 GStreamer backend's GL sink starts gstglcontext/gldisplay-event threads that are never joined on teardown, so allocating a MediaFile per ring leaked a pipeline and ~4 threads on every bell. A long-running instance accumulated 705 threads over ~4h of normal use. Cache one MediaFile per surface (priv.bell_media), rebuilt only when bell-audio-path changes and unref'd on dispose. Each bell now replays the same pipeline via seek(0)+play() instead of creating a new one. The notify::ended -> unref handler is removed: it was what discarded (and leaked) a pipeline per ring. seek(0) is required so an ended stream plays again (#8957). Verified on a real instance: GStreamer's global element counter reached only oggdemux4 over an hour of use (one pipeline per bell-ringing surface, reused) and thread count stayed flat, versus per-bell growth before. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -674,6 +674,12 @@ pub const Surface = extern struct {
|
||||
// false by a parent widget.
|
||||
bell_ringing: bool = false,
|
||||
|
||||
// The audio bell's MediaFile, reused across bells so we don't leak a
|
||||
// GStreamer pipeline (and its GL threads) on every ring. Built lazily
|
||||
// on the first audio bell and rebuilt when `bell-audio-path` changes;
|
||||
// unref'd on dispose. See ringBell and media.zig.
|
||||
bell_media: ?*gtk.MediaFile = null,
|
||||
|
||||
/// True if this surface is in an error state. This is currently
|
||||
/// a simple boolean with no additional information on WHAT the
|
||||
/// error state is, because we don't yet need it or use it. For now,
|
||||
@@ -1854,6 +1860,11 @@ pub const Surface = extern struct {
|
||||
priv.config = null;
|
||||
}
|
||||
|
||||
if (priv.bell_media) |v| {
|
||||
v.unref();
|
||||
priv.bell_media = null;
|
||||
}
|
||||
|
||||
if (priv.vadj_signal_group) |group| {
|
||||
group.setTarget(null);
|
||||
group.as(gobject.Object).unref();
|
||||
@@ -2486,8 +2497,15 @@ pub const Surface = extern struct {
|
||||
1.0,
|
||||
);
|
||||
|
||||
const media_file = media.fromFilename(path) orelse break :audio;
|
||||
media.playMediaFile(media_file, volume, required);
|
||||
// Reuse one MediaFile per surface (rebuilt only when the path
|
||||
// changes) so each bell replays the same pipeline instead of
|
||||
// leaking a fresh one. Assign unconditionally: bellMediaFile frees
|
||||
// any stale MediaFile and returns the current slot value (possibly
|
||||
// null if the path is now inaccessible), so priv.bell_media never
|
||||
// dangles.
|
||||
priv.bell_media = media.bellMediaFile(priv.bell_media, path, required);
|
||||
const media_file = priv.bell_media orelse break :audio;
|
||||
media.playBell(media_file, volume);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -44,9 +44,38 @@ pub fn fromResource(path: [:0]const u8) ?*gtk.MediaFile {
|
||||
return gtk.MediaFile.newForResource(path);
|
||||
}
|
||||
|
||||
pub fn playMediaFile(media_file: *gtk.MediaFile, volume: f64, required: bool) void {
|
||||
// If the audio file is marked as required, we'll emit an error if
|
||||
// there was a problem playing it. Otherwise there will be silence.
|
||||
/// Get-or-create a reusable bell MediaFile targeting `path`.
|
||||
///
|
||||
/// `current` is the surface's currently-cached MediaFile (or null). If it
|
||||
/// already targets `path` it is returned unchanged; otherwise it is unref'd and
|
||||
/// a fresh MediaFile is built for `path`. Returns null (after freeing `current`)
|
||||
/// if `path` is inaccessible, leaving the caller's slot empty.
|
||||
///
|
||||
/// Reusing one MediaFile per surface is what prevents the GStreamer pipeline
|
||||
/// leak: `gtk.MediaFile.newForFilename` spins up a full pipeline (and, via the
|
||||
/// GTK4 GStreamer backend's GL sink, gstglcontext/gldisplay-event threads) that
|
||||
/// is never torn down on the happy path, so allocating one per bell leaked a
|
||||
/// pipeline + its threads on every ring. See the caller in surface.zig.
|
||||
pub fn bellMediaFile(
|
||||
current: ?*gtk.MediaFile,
|
||||
path: [:0]const u8,
|
||||
required: bool,
|
||||
) ?*gtk.MediaFile {
|
||||
if (current) |media_file| {
|
||||
if (isForPath(media_file, path)) return media_file;
|
||||
media_file.unref();
|
||||
}
|
||||
|
||||
const media_file = fromFilename(path) orelse return null;
|
||||
|
||||
// If the audio file is marked as required, we'll emit an error if there
|
||||
// was a problem playing it. Otherwise there will be silence. We connect
|
||||
// this once, here, because the MediaFile is reused across bells.
|
||||
//
|
||||
// NOTE: we intentionally do NOT connect notify::ended to unref. The
|
||||
// MediaFile is owned by the surface and replayed via `seek(0)` for every
|
||||
// bell; unref'ing on `ended` is precisely what previously discarded (and
|
||||
// leaked) a pipeline per ring.
|
||||
if (required) {
|
||||
_ = gobject.Object.signals.notify.connect(
|
||||
media_file,
|
||||
@@ -57,21 +86,27 @@ pub fn playMediaFile(media_file: *gtk.MediaFile, volume: f64, required: bool) vo
|
||||
);
|
||||
}
|
||||
|
||||
// Watch for the "ended" signal so that we can clean up after
|
||||
// ourselves.
|
||||
_ = gobject.Object.signals.notify.connect(
|
||||
media_file,
|
||||
?*anyopaque,
|
||||
mediaFileEnded,
|
||||
null,
|
||||
.{ .detail = "ended" },
|
||||
);
|
||||
return media_file;
|
||||
}
|
||||
|
||||
/// (Re)play `media_file` at `volume`. `seek(0)` rewinds first so that a
|
||||
/// previously-ended stream plays again; without it playback only ever happens
|
||||
/// once (see #8957). Safe on a freshly-created stream as well.
|
||||
pub fn playBell(media_file: *gtk.MediaFile, volume: f64) void {
|
||||
const media_stream = media_file.as(gtk.MediaStream);
|
||||
media_stream.setVolume(volume);
|
||||
media_stream.seek(0);
|
||||
media_stream.play();
|
||||
}
|
||||
|
||||
/// Whether `media_file` was created for `path`.
|
||||
fn isForPath(media_file: *gtk.MediaFile, path: [:0]const u8) bool {
|
||||
const file = media_file.getFile() orelse return false;
|
||||
const cur = file.getPath() orelse return false;
|
||||
defer glib.free(cur);
|
||||
return std.mem.eql(u8, std.mem.span(cur), path);
|
||||
}
|
||||
|
||||
fn mediaFileError(
|
||||
media_file: *gtk.MediaFile,
|
||||
_: *gobject.ParamSpec,
|
||||
@@ -92,11 +127,3 @@ fn mediaFileError(
|
||||
err.f_message orelse "",
|
||||
});
|
||||
}
|
||||
|
||||
fn mediaFileEnded(
|
||||
media_file: *gtk.MediaFile,
|
||||
_: *gobject.ParamSpec,
|
||||
_: ?*anyopaque,
|
||||
) callconv(.c) void {
|
||||
media_file.unref();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user