fix(idle): avoid portal signal cleanup crash

commit: aa250abd7a7b191ee5a95a773c1e0fbbd3ec4d59
author: brice · 4mo ago · 2026-06-06 05:43 UTC
committer: brice · 4mo ago · 2026-06-06 05:57 UTC
parent: 88f46cc
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +113 -6

M .github/workflows/ci.yml +1 -1


@@ -83,7 +83,7 @@ jobs:
83	83		- name: Install dependencies
84	84		run: \|
85	85		pacman -Syu --noconfirm
86		-	pacman -S --noconfirm git go webkitgtk-6.0 gtk4 libadwaita gobject-introspection base-devel brotli
	86	+	pacman -S --noconfirm git go webkitgtk-6.0 gtk4 libadwaita gobject-introspection base-devel brotli dbus
87	87		- uses: actions/checkout@v6
88	88		- name: Cache Go modules
89	89		uses: actions/cache@v5


@@ -83,7 +83,7 @@ jobs:
83		- name: Install dependencies	83		- name: Install dependencies
84		run: \|	84		run: \|
85		pacman -Syu --noconfirm	85		pacman -Syu --noconfirm
86	-	pacman -S --noconfirm git go webkitgtk-6.0 gtk4 libadwaita gobject-introspection base-devel brotli	86	+	pacman -S --noconfirm git go webkitgtk-6.0 gtk4 libadwaita gobject-introspection base-devel brotli dbus
87		- uses: actions/checkout@v6	87		- uses: actions/checkout@v6
88		- name: Cache Go modules	88		- name: Cache Go modules
89		uses: actions/cache@v5	89		uses: actions/cache@v5

M internal/infrastructure/idle/portal_inhibitor.go +8 -5


@@ -135,7 +135,10 @@ func (p *PortalInhibitor) Inhibit(ctx context.Context, reason string) error {
135	135		func (p *PortalInhibitor) watchForResponse(ctx context.Context, handlePath dbus.ObjectPath) {
136	136		log := logging.FromContext(ctx)
137	137
138		-	if p.conn == nil {
	138	+	p.mu.Lock()
	139	+	conn := p.conn
	140	+	p.mu.Unlock()
	141	+	if conn == nil {
139	142		return
140	143		}
141	144

@@ -145,18 +148,18 @@ func (p *PortalInhibitor) watchForResponse(ctx context.Context, handlePath dbus.
145	148		requestIface, handlePath,
146	149		)
147	150
148		-	if err := p.conn.BusObject().Call("org.freedesktop.DBus.AddMatch", 0, matchRule).Err; err != nil {
	151	+	if err := conn.BusObject().Call("org.freedesktop.DBus.AddMatch", 0, matchRule).Err; err != nil {
149	152		log.Debug().Err(err).Msg("idle inhibitor: failed to add signal match")
150	153		return
151	154		}
152	155
153	156		// Create a channel to receive signals
154	157		signals := make(chan *dbus.Signal, 1)
155		-	p.conn.Signal(signals)
	158	+	conn.Signal(signals)
156	159
157	160		defer func() {
158		-	p.conn.RemoveSignal(signals)
159		-	_ = p.conn.BusObject().Call("org.freedesktop.DBus.RemoveMatch", 0, matchRule).Err
	161	+	conn.RemoveSignal(signals)
	162	+	_ = conn.BusObject().Call("org.freedesktop.DBus.RemoveMatch", 0, matchRule).Err
160	163		}()
161	164
162	165		// Wait for either a Response signal or context cancellation

 func (p *PortalInhibitor) watchForResponse(ctx context.Context, handlePath dbus.ObjectPath) {
 	log := logging.FromContext(ctx)
-	if p.conn == nil {
+	p.mu.Lock()
+	conn := p.conn
+	p.mu.Unlock()
+	if conn == nil {
 		return
 	}
 		requestIface, handlePath,
 	)
-	if err := p.conn.BusObject().Call("org.freedesktop.DBus.AddMatch", 0, matchRule).Err; err != nil {
+	if err := conn.BusObject().Call("org.freedesktop.DBus.AddMatch", 0, matchRule).Err; err != nil {
 		log.Debug().Err(err).Msg("idle inhibitor: failed to add signal match")
 		return
 	}
 	// Create a channel to receive signals
 	signals := make(chan *dbus.Signal, 1)
-	p.conn.Signal(signals)
+	conn.Signal(signals)
 	defer func() {
-		p.conn.RemoveSignal(signals)
+		conn.RemoveSignal(signals)
-		_ = p.conn.BusObject().Call("org.freedesktop.DBus.RemoveMatch", 0, matchRule).Err
+		_ = conn.BusObject().Call("org.freedesktop.DBus.RemoveMatch", 0, matchRule).Err
 	}()
 	// Wait for either a Response signal or context cancellation

A internal/infrastructure/idle/portal_inhibitor_test.go +104 -0

@@ -0,0 +1,104 @@
	1	+	package idle
	2	+
	3	+	import (
	4	+	"bufio"
	5	+	"bytes"
	6	+	"context"
	7	+	"os/exec"
	8	+	"strings"
	9	+	"sync"
	10	+	"testing"
	11	+
	12	+	"github.com/godbus/dbus/v5"
	13	+	)
	14	+
	15	+	func TestWatchForResponseCleanupSurvivesConnFieldCleared(t *testing.T) {
	16	+	// clearConnOnDoneContext clears inhibitor.conn after watchForResponse has
	17	+	// installed its cleanup defer; the test fails if cleanup dereferences the
	18	+	// mutable field instead of the D-Bus connection snapshot.
	19	+	conn := connectPrivateSessionBus(t)
	20	+	defer conn.Close()
	21	+
	22	+	inhibitor := &PortalInhibitor{conn: conn}
	23	+	ctx := &clearConnOnDoneContext{
	24	+	Context: context.Background(),
	25	+	done: closedDoneChannel(),
	26	+	clear: func() {
	27	+	inhibitor.mu.Lock()
	28	+	inhibitor.conn = nil
	29	+	inhibitor.mu.Unlock()
	30	+	},
	31	+	}
	32	+
	33	+	inhibitor.watchForResponse(ctx, "/org/freedesktop/portal/desktop/request/dumber/test")
	34	+
	35	+	inhibitor.mu.Lock()
	36	+	connCleared := inhibitor.conn == nil
	37	+	inhibitor.mu.Unlock()
	38	+	if !connCleared {
	39	+	t.Fatal("watchForResponse returned before observing context cancellation")
	40	+	}
	41	+	}
	42	+
	43	+	func connectPrivateSessionBus(t testing.T) dbus.Conn {
	44	+	t.Helper()
	45	+
	46	+	address := startPrivateSessionBus(t)
	47	+	conn, err := dbus.Connect(address)
	48	+	if err != nil {
	49	+	t.Fatalf("connect to private D-Bus session bus: %v", err)
	50	+	}
	51	+	return conn
	52	+	}
	53	+
	54	+	func startPrivateSessionBus(t *testing.T) string {
	55	+	t.Helper()
	56	+
	57	+	if _, err := exec.LookPath("dbus-daemon"); err != nil {
	58	+	t.Skipf("dbus-daemon unavailable: %v", err)
	59	+	}
	60	+
	61	+	var stderr bytes.Buffer
	62	+	cmd := exec.Command("dbus-daemon", "--session", "--nofork", "--print-address=1")
	63	+	cmd.Stderr = &stderr
	64	+
	65	+	stdout, err := cmd.StdoutPipe()
	66	+	if err != nil {
	67	+	t.Fatalf("open dbus-daemon stdout: %v", err)
	68	+	}
	69	+	err = cmd.Start()
	70	+	if err != nil {
	71	+	t.Fatalf("start dbus-daemon: %v", err)
	72	+	}
	73	+
	74	+	t.Cleanup(func() {
	75	+	if cmd.Process != nil {
	76	+	_ = cmd.Process.Kill()
	77	+	}
	78	+	_ = cmd.Wait()
	79	+	})
	80	+
	81	+	address, err := bufio.NewReader(stdout).ReadString('\n')
	82	+	if err != nil {
	83	+	t.Fatalf("read dbus-daemon address: %v; stderr: %s", err, stderr.String())
	84	+	}
	85	+	return strings.TrimSpace(address)
	86	+	}
	87	+
	88	+	type clearConnOnDoneContext struct {
	89	+	context.Context
	90	+	done <-chan struct{}
	91	+	once sync.Once
	92	+	clear func()
	93	+	}
	94	+
	95	+	func (c *clearConnOnDoneContext) Done() <-chan struct{} {
	96	+	c.once.Do(c.clear)
	97	+	return c.done
	98	+	}
	99	+
	100	+	func closedDoneChannel() <-chan struct{} {
	101	+	done := make(chan struct{})
	102	+	close(done)
	103	+	return done
	104	+	}

@@ -0,0 +1,104 @@
	1	+	package idle
	2	+
	3	+	import (
	4	+	"bufio"
	5	+	"bytes"
	6	+	"context"
	7	+	"os/exec"
	8	+	"strings"
	9	+	"sync"
	10	+	"testing"
	11	+
	12	+	"github.com/godbus/dbus/v5"
	13	+	)
	14	+
	15	+	func TestWatchForResponseCleanupSurvivesConnFieldCleared(t *testing.T) {
	16	+	// clearConnOnDoneContext clears inhibitor.conn after watchForResponse has
	17	+	// installed its cleanup defer; the test fails if cleanup dereferences the
	18	+	// mutable field instead of the D-Bus connection snapshot.
	19	+	conn := connectPrivateSessionBus(t)
	20	+	defer conn.Close()
	21	+
	22	+	inhibitor := &PortalInhibitor{conn: conn}
	23	+	ctx := &clearConnOnDoneContext{
	24	+	Context: context.Background(),
	25	+	done: closedDoneChannel(),
	26	+	clear: func() {
	27	+	inhibitor.mu.Lock()
	28	+	inhibitor.conn = nil
	29	+	inhibitor.mu.Unlock()
	30	+	},
	31	+	}
	32	+
	33	+	inhibitor.watchForResponse(ctx, "/org/freedesktop/portal/desktop/request/dumber/test")
	34	+
	35	+	inhibitor.mu.Lock()
	36	+	connCleared := inhibitor.conn == nil
	37	+	inhibitor.mu.Unlock()
	38	+	if !connCleared {
	39	+	t.Fatal("watchForResponse returned before observing context cancellation")
	40	+	}
	41	+	}
	42	+
	43	+	func connectPrivateSessionBus(t testing.T) dbus.Conn {
	44	+	t.Helper()
	45	+
	46	+	address := startPrivateSessionBus(t)
	47	+	conn, err := dbus.Connect(address)
	48	+	if err != nil {
	49	+	t.Fatalf("connect to private D-Bus session bus: %v", err)
	50	+	}
	51	+	return conn
	52	+	}
	53	+
	54	+	func startPrivateSessionBus(t *testing.T) string {
	55	+	t.Helper()
	56	+
	57	+	if _, err := exec.LookPath("dbus-daemon"); err != nil {
	58	+	t.Skipf("dbus-daemon unavailable: %v", err)
	59	+	}
	60	+
	61	+	var stderr bytes.Buffer
	62	+	cmd := exec.Command("dbus-daemon", "--session", "--nofork", "--print-address=1")
	63	+	cmd.Stderr = &stderr
	64	+
	65	+	stdout, err := cmd.StdoutPipe()
	66	+	if err != nil {
	67	+	t.Fatalf("open dbus-daemon stdout: %v", err)
	68	+	}
	69	+	err = cmd.Start()
	70	+	if err != nil {
	71	+	t.Fatalf("start dbus-daemon: %v", err)
	72	+	}
	73	+
	74	+	t.Cleanup(func() {
	75	+	if cmd.Process != nil {
	76	+	_ = cmd.Process.Kill()
	77	+	}
	78	+	_ = cmd.Wait()
	79	+	})
	80	+
	81	+	address, err := bufio.NewReader(stdout).ReadString('\n')
	82	+	if err != nil {
	83	+	t.Fatalf("read dbus-daemon address: %v; stderr: %s", err, stderr.String())
	84	+	}
	85	+	return strings.TrimSpace(address)
	86	+	}
	87	+
	88	+	type clearConnOnDoneContext struct {
	89	+	context.Context
	90	+	done <-chan struct{}
	91	+	once sync.Once
	92	+	clear func()
	93	+	}
	94	+
	95	+	func (c *clearConnOnDoneContext) Done() <-chan struct{} {
	96	+	c.once.Do(c.clear)
	97	+	return c.done
	98	+	}
	99	+
	100	+	func closedDoneChannel() <-chan struct{} {
	101	+	done := make(chan struct{})
	102	+	close(done)
	103	+	return done
	104	+	}