fix(xwayland): bound Wait on inherited stderr, log its tail, test group kill

commit: 854022286c0242c3bfab6838b6e312371761d73a
author: brice · 27d ago · 2026-09-25 13:02 UTC
parent: 996a519
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +78 -12

M internal/adapters/xwayland/xwayland.go +23 -7


@@ -10,6 +10,7 @@ import (
10	10		"os"
11	11		"os/exec"
12	12		"path/filepath"
	13	+	"slices"
13	14		"strconv"
14	15		"strings"
15	16		"sync"

@@ -211,12 +212,13 @@ func (d *Display) Run(ctx context.Context) error {
211	212		return err
212	213		}
213	214		start := time.Now()
214		-	err := d.serve(ctx)
	215	+	stderr := &logWriter{log: d.opts.Log}
	216	+	err := d.serve(ctx, stderr)
215	217		if ctx.Err() != nil {
216	218		return nil
217	219		}
218	220		if err != nil {
219		-	d.opts.Log.Warn().Err(err).Str("display", d.Name()).Msg("xwayland-satellite exited")
	221	+	d.opts.Log.Warn().Err(err).Strs("stderr", stderr.tail()).Str("display", d.Name()).Msg("xwayland-satellite exited")
220	222		} else {
221	223		d.opts.Log.Info().Str("display", d.Name()).Msg("xwayland-satellite exited")
222	224		}

@@ -260,7 +262,7 @@ func (d *Display) waitClient(ctx context.Context, wake int) error {
260	262		}
261	263
262	264		// serve runs the satellite on the sockets until it exits or ctx ends.
263		-	func (d *Display) serve(ctx context.Context) error {
	265	+	func (d Display) serve(ctx context.Context, stderr logWriter) error {
264	266		args := []string{d.Name()}
265	267		for i := range d.sockets {
266	268		args = append(args, "-listenfd", strconv.Itoa(3+i))

@@ -268,7 +270,9 @@ func (d *Display) serve(ctx context.Context) error {
268	270		cmd := exec.Command(d.opts.Binary, args...)
269	271		cmd.Env = d.opts.Env
270	272		cmd.ExtraFiles = d.sockets
271		-	cmd.Stderr = &logWriter{log: d.opts.Log}
	273	+	cmd.Stderr = stderr
	274	+	// Xwayland inherits the stderr pipe: Wait must not wait for it.
	275	+	cmd.WaitDelay = 2 * time.Second
272	276		cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
273	277		if err := cmd.Start(); err != nil {
274	278		return err

@@ -311,12 +315,20 @@ func (d *Display) dropPending() {
311	315		}
312	316		}
313	317
314		-	// logWriter logs the satellite's stderr line by line; exec copies it from
315		-	// a pipe on its own goroutine.
	318	+	// logWriter logs the satellite's stderr line by line and keeps the last
	319	+	// lines for the exit log; exec copies it from a pipe on its own goroutine.
316	320		type logWriter struct {
317	321		mu sync.Mutex
318	322		log zerowrap.Logger
319	323		line []byte
	324	+	last []string
	325	+	}
	326	+
	327	+	// tail is the last lines written.
	328	+	func (w *logWriter) tail() []string {
	329	+	w.mu.Lock()
	330	+	defer w.mu.Unlock()
	331	+	return slices.Clone(w.last)
320	332		}
321	333
322	334		func (w *logWriter) Write(p []byte) (int, error) {

@@ -329,7 +341,11 @@ func (w *logWriter) Write(p []byte) (int, error) {
329	341		break
330	342		}
331	343		if i > 0 {
332		-	w.log.Debug().Str("line", string(w.line[:i])).Msg("xwayland-satellite")
	344	+	line := string(w.line[:i])
	345	+	w.log.Debug().Str("line", line).Msg("xwayland-satellite")
	346	+	if w.last = append(w.last, line); len(w.last) > 5 {
	347	+	w.last = w.last[1:]
	348	+	}
333	349		}
334	350		w.line = w.line[i+1:]
335	351		}

 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
 			return err
 		}
 		start := time.Now()
-		err := d.serve(ctx)
+		stderr := &logWriter{log: d.opts.Log}
+		err := d.serve(ctx, stderr)
 		if ctx.Err() != nil {
 			return nil
 		}
 		if err != nil {
-			d.opts.Log.Warn().Err(err).Str("display", d.Name()).Msg("xwayland-satellite exited")
+			d.opts.Log.Warn().Err(err).Strs("stderr", stderr.tail()).Str("display", d.Name()).Msg("xwayland-satellite exited")
 		} else {
 			d.opts.Log.Info().Str("display", d.Name()).Msg("xwayland-satellite exited")
 		}
 }
 // serve runs the satellite on the sockets until it exits or ctx ends.
-func (d *Display) serve(ctx context.Context) error {
+func (d *Display) serve(ctx context.Context, stderr *logWriter) error {
 	args := []string{d.Name()}
 	for i := range d.sockets {
 		args = append(args, "-listenfd", strconv.Itoa(3+i))
 	cmd := exec.Command(d.opts.Binary, args...)
 	cmd.Env = d.opts.Env
 	cmd.ExtraFiles = d.sockets
-	cmd.Stderr = &logWriter{log: d.opts.Log}
+	cmd.Stderr = stderr
+	// Xwayland inherits the stderr pipe: Wait must not wait for it.
+	cmd.WaitDelay = 2 * time.Second
 	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
 	if err := cmd.Start(); err != nil {
 		return err
 	}
 }
-// logWriter logs the satellite's stderr line by line; exec copies it from
+// logWriter logs the satellite's stderr line by line and keeps the last
-// a pipe on its own goroutine.
+// lines for the exit log; exec copies it from a pipe on its own goroutine.
 type logWriter struct {
 	mu   sync.Mutex
 	log  zerowrap.Logger
 	line []byte
+	last []string
+}
+// tail is the last lines written.
+func (w *logWriter) tail() []string {
+	w.mu.Lock()
+	defer w.mu.Unlock()
+	return slices.Clone(w.last)
 }
 func (w *logWriter) Write(p []byte) (int, error) {
 			break
 		}
 		if i > 0 {
-			w.log.Debug().Str("line", string(w.line[:i])).Msg("xwayland-satellite")
+			line := string(w.line[:i])
+			w.log.Debug().Str("line", line).Msg("xwayland-satellite")
+			if w.last = append(w.last, line); len(w.last) > 5 {
+				w.last = w.last[1:]
+			}
 		}
 		w.line = w.line[i+1:]
 	}

M internal/adapters/xwayland/xwayland_test.go +55 -5


@@ -2,6 +2,7 @@ package xwayland
2	2
3	3		import (
4	4		"context"
	5	+	"errors"
5	6		"os"
6	7		"path/filepath"
7	8		"strconv"

@@ -62,7 +63,6 @@ func TestOpenPicksFreeDisplay(t *testing.T) {
62	63		}
63	64		}
64	65
65		-
66	66		// A satellite that exits without accepting drops the waiting client and is
67	67		// not respawned until another client connects.
68	68		func TestRunDropsClientsOfFailedSatellite(t *testing.T) {

@@ -123,8 +123,8 @@ func readRuns(t *testing.T, path string, want int) []string {
123	123		// Each connection starts the satellite once, with both sockets handed over.
124	124		func TestRunStartsSatelliteOnConnect(t *testing.T) {
125	125		opts := testOptions(t, "")
126		-	opts.Binary, _ = satelliteScript(t, opts.TmpDir)
127		-	_, runs := satelliteScript(t, opts.TmpDir)
	126	+	var runs string
	127	+	opts.Binary, runs = satelliteScript(t, opts.TmpDir)
128	128		opts.Abstract = true
129	129		d, err := Open(opts)
130	130		if err != nil {

@@ -176,6 +176,9 @@ func TestStaleLockReclaimed(t *testing.T) {
176	176		if d.Name() != ":0" {
177	177		t.Fatalf("display %s", d.Name())
178	178		}
	179	+	if b, _ := os.ReadFile(d.lock); strings.TrimSpace(string(b)) != strconv.Itoa(os.Getpid()) {
	180	+	t.Fatalf("lock holds %q", b)
	181	+	}
179	182		}
180	183
181	184		func TestDirChecks(t *testing.T) {

@@ -210,8 +213,55 @@ func TestSupported(t *testing.T) {
210	213		if !Supported(context.Background(), "true") \|\| Supported(context.Background(), "false") {
211	214		t.Fatal("exit status not honoured")
212	215		}
	216	+	hang := filepath.Join(t.TempDir(), "hang")
	217	+	if err := os.WriteFile(hang, []byte("#!/bin/sh\nexec sleep 10\n"), 0o755); err != nil {
	218	+	t.Fatal(err)
	219	+	}
213	220		start := time.Now()
214		-	if Supported(context.Background(), "sleep") && time.Since(start) > 3*time.Second {
215		-	t.Fatal("hang")
	221	+	if Supported(context.Background(), hang) \|\| time.Since(start) > 3*time.Second {
	222	+	t.Fatalf("hanging binary: %v", time.Since(start))
	223	+	}
	224	+	}
	225	+
	226	+	// Stopping kills the satellite's whole process group, even when a child
	227	+	// keeps its stderr open.
	228	+	func TestRunKillsSatelliteGroup(t *testing.T) {
	229	+	opts := testOptions(t, "")
	230	+	pidfile := filepath.Join(opts.TmpDir, "pid")
	231	+	opts.Binary = filepath.Join(opts.TmpDir, "satellite")
	232	+	script := "#!/bin/sh\ntrap '' TERM\nsleep 30 &\necho $! > " + pidfile + "\nwait\n"
	233	+	if err := os.WriteFile(opts.Binary, []byte(script), 0o755); err != nil {
	234	+	t.Fatal(err)
	235	+	}
	236	+	d, err := Open(opts)
	237	+	if err != nil {
	238	+	t.Fatal(err)
	239	+	}
	240	+	defer d.Close()
	241	+	ctx, cancel := context.WithCancel(context.Background())
	242	+	done := make(chan error, 1)
	243	+	go func() { done <- d.Run(ctx) }()
	244	+	unix.Close(dial(t, d.socket))
	245	+	var pid int
	246	+	for deadline := time.Now().Add(3 * time.Second); pid == 0 && time.Now().Before(deadline); time.Sleep(20 * time.Millisecond) {
	247	+	b, _ := os.ReadFile(pidfile)
	248	+	pid, _ = strconv.Atoi(strings.TrimSpace(string(b)))
	249	+	}
	250	+	if pid == 0 {
	251	+	t.Fatal("satellite did not start")
	252	+	}
	253	+	cancel()
	254	+	select {
	255	+	case <-done:
	256	+	case <-time.After(5 * time.Second):
	257	+	t.Fatal("Run did not stop")
	258	+	}
	259	+	for deadline := time.Now().Add(2 * time.Second); ; time.Sleep(20 * time.Millisecond) {
	260	+	if errors.Is(unix.Kill(pid, 0), unix.ESRCH) {
	261	+	break
	262	+	}
	263	+	if time.Now().After(deadline) {
	264	+	t.Fatalf("child %d survived", pid)
	265	+	}
216	266		}
217	267		}


@@ -2,6 +2,7 @@ package xwayland
2			2
3		import (	3		import (
4		"context"	4		"context"
			5	+	"errors"
5		"os"	6		"os"
6		"path/filepath"	7		"path/filepath"
7		"strconv"	8		"strconv"

@@ -62,7 +63,6 @@ func TestOpenPicksFreeDisplay(t *testing.T) {
62		}	63		}
63		}	64		}
64			65
65	-
66		// A satellite that exits without accepting drops the waiting client and is	66		// A satellite that exits without accepting drops the waiting client and is
67		// not respawned until another client connects.	67		// not respawned until another client connects.
68		func TestRunDropsClientsOfFailedSatellite(t *testing.T) {	68		func TestRunDropsClientsOfFailedSatellite(t *testing.T) {

@@ -123,8 +123,8 @@ func readRuns(t *testing.T, path string, want int) []string {
123		// Each connection starts the satellite once, with both sockets handed over.	123		// Each connection starts the satellite once, with both sockets handed over.
124		func TestRunStartsSatelliteOnConnect(t *testing.T) {	124		func TestRunStartsSatelliteOnConnect(t *testing.T) {
125		opts := testOptions(t, "")	125		opts := testOptions(t, "")
126	-	opts.Binary, _ = satelliteScript(t, opts.TmpDir)	126	+	var runs string
127	-	_, runs := satelliteScript(t, opts.TmpDir)	127	+	opts.Binary, runs = satelliteScript(t, opts.TmpDir)
128		opts.Abstract = true	128		opts.Abstract = true
129		d, err := Open(opts)	129		d, err := Open(opts)
130		if err != nil {	130		if err != nil {

@@ -176,6 +176,9 @@ func TestStaleLockReclaimed(t *testing.T) {
176		if d.Name() != ":0" {	176		if d.Name() != ":0" {
177		t.Fatalf("display %s", d.Name())	177		t.Fatalf("display %s", d.Name())
178		}	178		}
			179	+	if b, _ := os.ReadFile(d.lock); strings.TrimSpace(string(b)) != strconv.Itoa(os.Getpid()) {
			180	+	t.Fatalf("lock holds %q", b)
			181	+	}
179		}	182		}
180			183
181		func TestDirChecks(t *testing.T) {	184		func TestDirChecks(t *testing.T) {

@@ -210,8 +213,55 @@ func TestSupported(t *testing.T) {
210		if !Supported(context.Background(), "true") \|\| Supported(context.Background(), "false") {	213		if !Supported(context.Background(), "true") \|\| Supported(context.Background(), "false") {
211		t.Fatal("exit status not honoured")	214		t.Fatal("exit status not honoured")
212		}	215		}
			216	+	hang := filepath.Join(t.TempDir(), "hang")
			217	+	if err := os.WriteFile(hang, []byte("#!/bin/sh\nexec sleep 10\n"), 0o755); err != nil {
			218	+	t.Fatal(err)
			219	+	}
213		start := time.Now()	220		start := time.Now()
214	-	if Supported(context.Background(), "sleep") && time.Since(start) > 3*time.Second {	221	+	if Supported(context.Background(), hang) \|\| time.Since(start) > 3*time.Second {
215	-	t.Fatal("hang")	222	+	t.Fatalf("hanging binary: %v", time.Since(start))
			223	+	}
			224	+	}
			225	+
			226	+	// Stopping kills the satellite's whole process group, even when a child
			227	+	// keeps its stderr open.
			228	+	func TestRunKillsSatelliteGroup(t *testing.T) {
			229	+	opts := testOptions(t, "")
			230	+	pidfile := filepath.Join(opts.TmpDir, "pid")
			231	+	opts.Binary = filepath.Join(opts.TmpDir, "satellite")
			232	+	script := "#!/bin/sh\ntrap '' TERM\nsleep 30 &\necho $! > " + pidfile + "\nwait\n"
			233	+	if err := os.WriteFile(opts.Binary, []byte(script), 0o755); err != nil {
			234	+	t.Fatal(err)
			235	+	}
			236	+	d, err := Open(opts)
			237	+	if err != nil {
			238	+	t.Fatal(err)
			239	+	}
			240	+	defer d.Close()
			241	+	ctx, cancel := context.WithCancel(context.Background())
			242	+	done := make(chan error, 1)
			243	+	go func() { done <- d.Run(ctx) }()
			244	+	unix.Close(dial(t, d.socket))
			245	+	var pid int
			246	+	for deadline := time.Now().Add(3 * time.Second); pid == 0 && time.Now().Before(deadline); time.Sleep(20 * time.Millisecond) {
			247	+	b, _ := os.ReadFile(pidfile)
			248	+	pid, _ = strconv.Atoi(strings.TrimSpace(string(b)))
			249	+	}
			250	+	if pid == 0 {
			251	+	t.Fatal("satellite did not start")
			252	+	}
			253	+	cancel()
			254	+	select {
			255	+	case <-done:
			256	+	case <-time.After(5 * time.Second):
			257	+	t.Fatal("Run did not stop")
			258	+	}
			259	+	for deadline := time.Now().Add(2 * time.Second); ; time.Sleep(20 * time.Millisecond) {
			260	+	if errors.Is(unix.Kill(pid, 0), unix.ESRCH) {
			261	+	break
			262	+	}
			263	+	if time.Now().After(deadline) {
			264	+	t.Fatalf("child %d survived", pid)
			265	+	}
216		}	266		}
217		}	267		}