a7ac56a fix(wayland): never block the display goroutine on events and clean up failed setup · neferwl

M internal/adapters/launcher/launcher.go +28 -1


@@ -2,7 +2,10 @@ package launcher
2	2
3	3		import (
4	4		"context"
	5	+	"fmt"
	6	+	"os"
5	7		"os/exec"
	8	+	"path/filepath"
6	9		"sort"
7	10		"strings"
8	11		"syscall"

@@ -61,7 +64,7 @@ func (l *Launcher) Run(ctx context.Context, reqs <-chan ports.SpawnRequest) erro
61	64		l.log.Warn().Msg("empty spawn argv")
62	65		continue
63	66		}
64		-	path, err := exec.LookPath(req.Argv[0])
	67	+	path, err := childLookPath(req.Argv[0], l.env)
65	68		if err != nil {
66	69		l.log.Warn().Err(err).Str("binary", req.Argv[0]).Msg("spawn failed")
67	70		continue

@@ -76,7 +79,31 @@ func (l *Launcher) Run(ctx context.Context, reqs <-chan ports.SpawnRequest) erro
76	79		l.log.Warn().Err(err).Str("binary", req.Argv[0]).Msg("spawn failed")
77	80		continue
78	81		}
	82	+	// Children have independent sessions and can outlive Run; reaping continues until process exit.
79	83		go func() { l.log.Debug().Err(cmd.Wait()).Str("binary", req.Argv[0]).Msg("child exited") }()
80	84		}
81	85		}
82	86		}
	87	+
	88	+	func childLookPath(name string, env []string) (string, error) {
	89	+	if strings.Contains(name, "/") {
	90	+	return name, nil
	91	+	}
	92	+	path := ""
	93	+	for _, entry := range env {
	94	+	if strings.HasPrefix(entry, "PATH=") {
	95	+	path = strings.TrimPrefix(entry, "PATH=")
	96	+	}
	97	+	}
	98	+	for _, dir := range filepath.SplitList(path) {
	99	+	if dir == "" {
	100	+	dir = "."
	101	+	}
	102	+	candidate := filepath.Join(dir, name)
	103	+	info, err := os.Stat(candidate)
	104	+	if err == nil && !info.IsDir() && info.Mode().Perm()&0111 != 0 {
	105	+	return candidate, nil
	106	+	}
	107	+	}
	108	+	return "", fmt.Errorf("executable %q not found in child PATH", name)
	109	+	}


@@ -2,7 +2,10 @@ package launcher
2			2
3		import (	3		import (
4		"context"	4		"context"
			5	+	"fmt"
			6	+	"os"
5		"os/exec"	7		"os/exec"
			8	+	"path/filepath"
6		"sort"	9		"sort"
7		"strings"	10		"strings"
8		"syscall"	11		"syscall"

@@ -61,7 +64,7 @@ func (l *Launcher) Run(ctx context.Context, reqs <-chan ports.SpawnRequest) erro
61		l.log.Warn().Msg("empty spawn argv")	64		l.log.Warn().Msg("empty spawn argv")
62		continue	65		continue
63		}	66		}
64	-	path, err := exec.LookPath(req.Argv[0])	67	+	path, err := childLookPath(req.Argv[0], l.env)
65		if err != nil {	68		if err != nil {
66		l.log.Warn().Err(err).Str("binary", req.Argv[0]).Msg("spawn failed")	69		l.log.Warn().Err(err).Str("binary", req.Argv[0]).Msg("spawn failed")
67		continue	70		continue

@@ -76,7 +79,31 @@ func (l *Launcher) Run(ctx context.Context, reqs <-chan ports.SpawnRequest) erro
76		l.log.Warn().Err(err).Str("binary", req.Argv[0]).Msg("spawn failed")	79		l.log.Warn().Err(err).Str("binary", req.Argv[0]).Msg("spawn failed")
77		continue	80		continue
78		}	81		}
			82	+	// Children have independent sessions and can outlive Run; reaping continues until process exit.
79		go func() { l.log.Debug().Err(cmd.Wait()).Str("binary", req.Argv[0]).Msg("child exited") }()	83		go func() { l.log.Debug().Err(cmd.Wait()).Str("binary", req.Argv[0]).Msg("child exited") }()
80		}	84		}
81		}	85		}
82		}	86		}
			87	+
			88	+	func childLookPath(name string, env []string) (string, error) {
			89	+	if strings.Contains(name, "/") {
			90	+	return name, nil
			91	+	}
			92	+	path := ""
			93	+	for _, entry := range env {
			94	+	if strings.HasPrefix(entry, "PATH=") {
			95	+	path = strings.TrimPrefix(entry, "PATH=")
			96	+	}
			97	+	}
			98	+	for _, dir := range filepath.SplitList(path) {
			99	+	if dir == "" {
			100	+	dir = "."
			101	+	}
			102	+	candidate := filepath.Join(dir, name)
			103	+	info, err := os.Stat(candidate)
			104	+	if err == nil && !info.IsDir() && info.Mode().Perm()&0111 != 0 {
			105	+	return candidate, nil
			106	+	}
			107	+	}
			108	+	return "", fmt.Errorf("executable %q not found in child PATH", name)
			109	+	}

M internal/adapters/launcher/launcher_test.go +35 -0


@@ -57,3 +57,38 @@ func TestRun(t *testing.T) {
57	57		t.Fatal("launcher did not stop")
58	58		}
59	59		}
	60	+
	61	+	func TestChildPath(t *testing.T) {
	62	+	dir := t.TempDir()
	63	+	name := "nefertty-child-only-script"
	64	+	result := filepath.Join(dir, "result")
	65	+	if err := os.WriteFile(filepath.Join(dir, name), []byte("#!/bin/sh\nprintf ok > \"$1\"\n"), 0700); err != nil {
	66	+	t.Fatal(err)
	67	+	}
	68	+	ctx, cancel := context.WithCancel(context.Background())
	69	+	defer cancel()
	70	+	reqs := make(chan ports.SpawnRequest, 1)
	71	+	done := make(chan error, 1)
	72	+	go func() { done <- New([]string{"PATH=" + dir}, logging.For(ctx, "launcher")).Run(ctx, reqs) }()
	73	+	reqs <- ports.SpawnRequest{Argv: []string{name, result}}
	74	+	deadline := time.After(3 * time.Second)
	75	+	for {
	76	+	if data, err := os.ReadFile(result); err == nil {
	77	+	if string(data) != "ok" {
	78	+	t.Fatalf("result: %q", data)
	79	+	}
	80	+	break
	81	+	}
	82	+	select {
	83	+	case <-deadline:
	84	+	t.Fatal("child not started")
	85	+	case <-time.After(10 * time.Millisecond):
	86	+	}
	87	+	}
	88	+	cancel()
	89	+	select {
	90	+	case <-done:
	91	+	case <-time.After(time.Second):
	92	+	t.Fatal("launcher hung")
	93	+	}
	94	+	}


@@ -57,3 +57,38 @@ func TestRun(t *testing.T) {
57	t.Fatal("launcher did not stop")	57		t.Fatal("launcher did not stop")
58	}	58		}
59	}	59		}
		60	+
		61	+	func TestChildPath(t *testing.T) {
		62	+	dir := t.TempDir()
		63	+	name := "nefertty-child-only-script"
		64	+	result := filepath.Join(dir, "result")
		65	+	if err := os.WriteFile(filepath.Join(dir, name), []byte("#!/bin/sh\nprintf ok > \"$1\"\n"), 0700); err != nil {
		66	+	t.Fatal(err)
		67	+	}
		68	+	ctx, cancel := context.WithCancel(context.Background())
		69	+	defer cancel()
		70	+	reqs := make(chan ports.SpawnRequest, 1)
		71	+	done := make(chan error, 1)
		72	+	go func() { done <- New([]string{"PATH=" + dir}, logging.For(ctx, "launcher")).Run(ctx, reqs) }()
		73	+	reqs <- ports.SpawnRequest{Argv: []string{name, result}}
		74	+	deadline := time.After(3 * time.Second)
		75	+	for {
		76	+	if data, err := os.ReadFile(result); err == nil {
		77	+	if string(data) != "ok" {
		78	+	t.Fatalf("result: %q", data)
		79	+	}
		80	+	break
		81	+	}
		82	+	select {
		83	+	case <-deadline:
		84	+	t.Fatal("child not started")
		85	+	case <-time.After(10 * time.Millisecond):
		86	+	}
		87	+	}
		88	+	cancel()
		89	+	select {
		90	+	case <-done:
		91	+	case <-time.After(time.Second):
		92	+	t.Fatal("launcher hung")
		93	+	}
		94	+	}

M internal/adapters/wayland/server.go +40 -5


@@ -21,7 +21,7 @@ type Options struct {
21	21		OutputWidth, OutputHeight int
22	22		}
23	23
24		-	// Channels carries client notifications and commands. Events must be buffered.
	24	+	// Channels carries client notifications and commands. Events may be unbuffered.
25	25		type Channels struct {
26	26		Events chan<- ports.ClientEvent
27	27		Commands <-chan ports.ClientCommand

@@ -41,6 +41,10 @@ type Server struct {
41	41		windows map[ports.WindowID]*window
42	42		nextWindow ports.WindowID
43	43		focused ports.WindowID
	44	+	// eventMu protects only the notification queue, not display-owned window state.
	45	+	eventMu sync.Mutex
	46	+	events []ports.ClientEvent
	47	+	eventReady chan struct{}
44	48		}
45	49
46	50		func New(opts Options, ch Channels, log zerowrap.Logger) (*Server, error) {

@@ -56,16 +60,19 @@ func New(opts Options, ch Channels, log zerowrap.Logger) (*Server, error) {
56	60		}
57	61		name, fd, cleanup, err := listen(opts.RuntimeDir)
58	62		if err != nil {
	63	+	d.Close()
59	64		return nil, err
60	65		}
61	66		if err = d.AddSocketFD(fd); err != nil {
62	67		unix.Close(fd)
63	68		cleanup()
	69	+	d.Close()
64	70		return nil, err
65	71		}
66		-	s := &Server{display: d, name: name, cleanup: cleanup, log: log, channels: ch, surfaces: make(map[server.Resource]surface), windows: make(map[ports.WindowID]*window), nextWindow: 1}
	72	+	s := &Server{display: d, name: name, cleanup: cleanup, log: log, channels: ch, surfaces: make(map[server.Resource]surface), windows: make(map[ports.WindowID]*window), nextWindow: 1, eventReady: make(chan struct{}, 1)}
67	73		if err = registerGlobals(d, opts, s); err != nil {
68	74		cleanup()
	75	+	d.Close()
69	76		return nil, err
70	77		}
71	78		return s, nil

@@ -77,7 +84,8 @@ func (s *Server) Run(ctx context.Context) error {
77	84		s.started = time.Now()
78	85		s.ctx = ctx
79	86		var wg sync.WaitGroup
80		-	wg.Add(2)
	87	+	wg.Add(3)
	88	+	go func() { defer wg.Done(); s.forward(ctx) }()
81	89		go func() {
82	90		defer wg.Done()
83	91		for {

@@ -146,9 +154,36 @@ func (s *Server) emit(ev ports.ClientEvent) {
146	154		if s.channels.Events == nil {
147	155		return
148	156		}
	157	+	s.eventMu.Lock()
	158	+	s.events = append(s.events, ev)
	159	+	s.eventMu.Unlock()
149	160		select {
150		-	case s.channels.Events <- ev:
151		-	case <-s.ctx.Done():
	161	+	case s.eventReady <- struct{}{}:
	162	+	default:
	163	+	}
	164	+	}
	165	+
	166	+	func (s *Server) forward(ctx context.Context) {
	167	+	for {
	168	+	s.eventMu.Lock()
	169	+	if len(s.events) == 0 {
	170	+	s.eventMu.Unlock()
	171	+	select {
	172	+	case <-ctx.Done():
	173	+	return
	174	+	case <-s.eventReady:
	175	+	continue
	176	+	}
	177	+	}
	178	+	ev := s.events[0]
	179	+	s.events[0] = nil
	180	+	s.events = s.events[1:]
	181	+	s.eventMu.Unlock()
	182	+	select {
	183	+	case <-ctx.Done():
	184	+	return
	185	+	case s.channels.Events <- ev:
	186	+	}
152	187		}
153	188		}
154	189

 	OutputWidth, OutputHeight int
 }
-// Channels carries client notifications and commands. Events must be buffered.
+// Channels carries client notifications and commands. Events may be unbuffered.
 type Channels struct {
 	Events   chan<- ports.ClientEvent
 	Commands <-chan ports.ClientCommand
 	windows    map[ports.WindowID]*window
 	nextWindow ports.WindowID
 	focused    ports.WindowID
+	// eventMu protects only the notification queue, not display-owned window state.
+	eventMu    sync.Mutex
+	events     []ports.ClientEvent
+	eventReady chan struct{}
 }
 func New(opts Options, ch Channels, log zerowrap.Logger) (*Server, error) {
 	}
 	name, fd, cleanup, err := listen(opts.RuntimeDir)
 	if err != nil {
+		d.Close()
 		return nil, err
 	}
 	if err = d.AddSocketFD(fd); err != nil {
 		unix.Close(fd)
 		cleanup()
+		d.Close()
 		return nil, err
 	}
-	s := &Server{display: d, name: name, cleanup: cleanup, log: log, channels: ch, surfaces: make(map[*server.Resource]*surface), windows: make(map[ports.WindowID]*window), nextWindow: 1}
+	s := &Server{display: d, name: name, cleanup: cleanup, log: log, channels: ch, surfaces: make(map[*server.Resource]*surface), windows: make(map[ports.WindowID]*window), nextWindow: 1, eventReady: make(chan struct{}, 1)}
 	if err = registerGlobals(d, opts, s); err != nil {
 		cleanup()
+		d.Close()
 		return nil, err
 	}
 	return s, nil
 	s.started = time.Now()
 	s.ctx = ctx
 	var wg sync.WaitGroup
-	wg.Add(2)
+	wg.Add(3)
+	go func() { defer wg.Done(); s.forward(ctx) }()
 	go func() {
 		defer wg.Done()
 		for {
 	if s.channels.Events == nil {
 		return
 	}
+	s.eventMu.Lock()
+	s.events = append(s.events, ev)
+	s.eventMu.Unlock()
 	select {
-	case s.channels.Events <- ev:
+	case s.eventReady <- struct{}{}:
-	case <-s.ctx.Done():
+	default:
+	}
+}
+func (s *Server) forward(ctx context.Context) {
+	for {
+		s.eventMu.Lock()
+		if len(s.events) == 0 {
+			s.eventMu.Unlock()
+			select {
+			case <-ctx.Done():
+				return
+			case <-s.eventReady:
+				continue
+			}
+		}
+		ev := s.events[0]
+		s.events[0] = nil
+		s.events = s.events[1:]
+		s.eventMu.Unlock()
+		select {
+		case <-ctx.Done():
+			return
+		case s.channels.Events <- ev:
+		}
 	}
 }

M internal/adapters/wayland/server_test.go +75 -0


@@ -289,3 +289,78 @@ func TestFootWindow(t *testing.T) {
289	289		t.Fatalf("foot did not map: %s", output.String())
290	290		}
291	291		}
	292	+
	293	+	func TestNewMissingRuntimeDirClosesDisplay(t *testing.T) {
	294	+	before, err := os.ReadDir("/proc/self/fd")
	295	+	if err != nil {
	296	+	t.Skip(err)
	297	+	}
	298	+	dir := filepath.Join(t.TempDir(), "missing")
	299	+	if _, err := New(Options{RuntimeDir: dir}, Channels{}, logging.For(context.Background(), "wayland")); err == nil {
	300	+	t.Fatal("expected error")
	301	+	}
	302	+	after, err := os.ReadDir("/proc/self/fd")
	303	+	if err != nil {
	304	+	t.Fatal(err)
	305	+	}
	306	+	if len(after) != len(before) {
	307	+	t.Fatalf("fd leak: %d -> %d", len(before), len(after))
	308	+	}
	309	+	if _, err := os.Stat(dir); !os.IsNotExist(err) {
	310	+	t.Fatalf("unexpected directory: %v", err)
	311	+	}
	312	+	}
	313	+
	314	+	func TestUnbufferedEventsDoNotBlockDisplay(t *testing.T) {
	315	+	tool, err := exec.LookPath("weston-simple-shm")
	316	+	if err != nil {
	317	+	t.Skip("weston-simple-shm not installed")
	318	+	}
	319	+	dir := t.TempDir()
	320	+	events := make(chan ports.ClientEvent)
	321	+	s, err := New(Options{RuntimeDir: dir, OutputWidth: 1920, OutputHeight: 1080}, Channels{Events: events}, logging.For(context.Background(), "wayland"))
	322	+	if err != nil {
	323	+	t.Fatal(err)
	324	+	}
	325	+	ctx, cancel := context.WithCancel(context.Background())
	326	+	done := make(chan error, 1)
	327	+	go func() { done <- s.Run(ctx) }()
	328	+	t.Cleanup(func() {
	329	+	cancel()
	330	+	select {
	331	+	case err := <-done:
	332	+	if err != nil {
	333	+	t.Error(err)
	334	+	}
	335	+	case <-time.After(2 * time.Second):
	336	+	t.Error("Run hung")
	337	+	}
	338	+	})
	339	+	lifecycleClient(t, tool, s, dir, nil)
	340	+	deadline := time.After(5 * time.Second)
	341	+	for {
	342	+	ready := make(chan bool, 1)
	343	+	go func() { ready <- s.display.Do(func() {}) }()
	344	+	select {
	345	+	case ok := <-ready:
	346	+	if !ok {
	347	+	t.Fatal("display stopped")
	348	+	}
	349	+	case <-time.After(time.Second):
	350	+	t.Fatal("display blocked on event")
	351	+	}
	352	+	mappedOnDisplay := false
	353	+	if !s.display.Do(func() { mappedOnDisplay = len(s.windows) > 0 }) {
	354	+	t.Fatal("display stopped")
	355	+	}
	356	+	if mappedOnDisplay {
	357	+	break
	358	+	}
	359	+	select {
	360	+	case <-deadline:
	361	+	t.Fatal("no mapping event")
	362	+	case <-time.After(10 * time.Millisecond):
	363	+	}
	364	+	}
	365	+	mapped(t, events, time.Second)
	366	+	}


@@ -289,3 +289,78 @@ func TestFootWindow(t *testing.T) {
289	t.Fatalf("foot did not map: %s", output.String())	289		t.Fatalf("foot did not map: %s", output.String())
290	}	290		}
291	}	291		}
		292	+
		293	+	func TestNewMissingRuntimeDirClosesDisplay(t *testing.T) {
		294	+	before, err := os.ReadDir("/proc/self/fd")
		295	+	if err != nil {
		296	+	t.Skip(err)
		297	+	}
		298	+	dir := filepath.Join(t.TempDir(), "missing")
		299	+	if _, err := New(Options{RuntimeDir: dir}, Channels{}, logging.For(context.Background(), "wayland")); err == nil {
		300	+	t.Fatal("expected error")
		301	+	}
		302	+	after, err := os.ReadDir("/proc/self/fd")
		303	+	if err != nil {
		304	+	t.Fatal(err)
		305	+	}
		306	+	if len(after) != len(before) {
		307	+	t.Fatalf("fd leak: %d -> %d", len(before), len(after))
		308	+	}
		309	+	if _, err := os.Stat(dir); !os.IsNotExist(err) {
		310	+	t.Fatalf("unexpected directory: %v", err)
		311	+	}
		312	+	}
		313	+
		314	+	func TestUnbufferedEventsDoNotBlockDisplay(t *testing.T) {
		315	+	tool, err := exec.LookPath("weston-simple-shm")
		316	+	if err != nil {
		317	+	t.Skip("weston-simple-shm not installed")
		318	+	}
		319	+	dir := t.TempDir()
		320	+	events := make(chan ports.ClientEvent)
		321	+	s, err := New(Options{RuntimeDir: dir, OutputWidth: 1920, OutputHeight: 1080}, Channels{Events: events}, logging.For(context.Background(), "wayland"))
		322	+	if err != nil {
		323	+	t.Fatal(err)
		324	+	}
		325	+	ctx, cancel := context.WithCancel(context.Background())
		326	+	done := make(chan error, 1)
		327	+	go func() { done <- s.Run(ctx) }()
		328	+	t.Cleanup(func() {
		329	+	cancel()
		330	+	select {
		331	+	case err := <-done:
		332	+	if err != nil {
		333	+	t.Error(err)
		334	+	}
		335	+	case <-time.After(2 * time.Second):
		336	+	t.Error("Run hung")
		337	+	}
		338	+	})
		339	+	lifecycleClient(t, tool, s, dir, nil)
		340	+	deadline := time.After(5 * time.Second)
		341	+	for {
		342	+	ready := make(chan bool, 1)
		343	+	go func() { ready <- s.display.Do(func() {}) }()
		344	+	select {
		345	+	case ok := <-ready:
		346	+	if !ok {
		347	+	t.Fatal("display stopped")
		348	+	}
		349	+	case <-time.After(time.Second):
		350	+	t.Fatal("display blocked on event")
		351	+	}
		352	+	mappedOnDisplay := false
		353	+	if !s.display.Do(func() { mappedOnDisplay = len(s.windows) > 0 }) {
		354	+	t.Fatal("display stopped")
		355	+	}
		356	+	if mappedOnDisplay {
		357	+	break
		358	+	}
		359	+	select {
		360	+	case <-deadline:
		361	+	t.Fatal("no mapping event")
		362	+	case <-time.After(10 * time.Millisecond):
		363	+	}
		364	+	}
		365	+	mapped(t, events, time.Second)
		366	+	}