fix(core): keep Display usable after deadlines and run every global handler

- A read deadline no longer poisons the connection: os.ErrDeadlineExceeded
  is returned without being stored as a sticky read error.
- Registry.AddHandler and AddGlobalHandler accumulate handlers instead of
  replacing the previous one for the same interface or "*".
- marshalArg rejects plain Go int arguments instead of writing zero.
- Dispatch documents that handlers must not call Dispatch/Roundtrip on the
  same Display.

commit: 6cc258ac466a7bbdfaacd9944d5868fbc9eae8a2
author: brice · 23d ago · 2026-09-29 15:42 UTC
parent: f4254c0
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +91 -19

M client.go +23 -19


@@ -101,7 +101,7 @@ func newDisplay(conn net.Conn) *Display {
101	101		id: d.allocateID(),
102	102		display: d,
103	103		globals: make(map[uint32]Global),
104		-	handlers: make(map[string]GlobalHandler),
	104	+	handlers: make(map[string][]GlobalHandler),
105	105		}
106	106		d.objects.Store(d.registry.id, d.registry)
107	107		return d

@@ -113,7 +113,7 @@ type Registry struct {
113	113		display *Display
114	114		globals map[uint32]Global
115	115		mu sync.RWMutex
116		-	handlers map[string]GlobalHandler
	116	+	handlers map[string][]GlobalHandler
117	117		removeHandlers []RegistryGlobalRemoveHandler
118	118		}
119	119

@@ -337,10 +337,6 @@ func (d Display) marshalArg(buf bytes.Buffer, arg interface{}) error {
337	337		case nil:
338	338		// Null object
339	339		return binary.Write(buf, binary.LittleEndian, uint32(0))
340		-	case int:
341		-	// File descriptor - write placeholder in message
342		-	// Actual FD will be sent via SCM_RIGHTS
343		-	return binary.Write(buf, binary.LittleEndian, uint32(0))
344	340		case uintptr:
345	341		// File descriptor as uintptr - NO placeholder in message for neurlang compatibility
346	342		// FD is ONLY sent via SCM_RIGHTS, not in the message body

@@ -357,7 +353,12 @@ func (d Display) marshalArg(buf bytes.Buffer, arg interface{}) error {
357	353		// Read boundaries never define message boundaries: a header split across
358	354		// several reads and several messages arriving in one read are both handled.
359	355		// Dispatch returns net.ErrClosed after Close, a typed ProtocolError for a
360		-	// malformed frame, and io.ErrUnexpectedEOF when the peer closes mid-frame.
	356	+	// malformed frame, io.ErrUnexpectedEOF when the peer closes mid-frame, and
	357	+	// os.ErrDeadlineExceeded (not sticky) when a read deadline expires.
	358	+	//
	359	+	// Event handlers run while Dispatch holds the dispatch lock, so a handler must
	360	+	// not call Dispatch or Roundtrip on the same Display: that call deadlocks.
	361	+	// Hand work that needs a roundtrip to another goroutine instead.
361	362		func (d *Display) Dispatch() error {
362	363		d.dispatchMu.Lock()
363	364		defer d.dispatchMu.Unlock()

@@ -457,6 +458,11 @@ func (d *Display) fill() error {
457	458		if errors.Is(err, io.EOF) {
458	459		err = io.ErrUnexpectedEOF
459	460		}
	461	+	// A missed deadline is not a broken stream: report it without making
	462	+	// it sticky, so a later Dispatch can read again.
	463	+	if errors.Is(err, os.ErrDeadlineExceeded) {
	464	+	return err
	465	+	}
460	466		d.readErr = err
461	467		// A control-message failure corrupts the descriptor stream, so it is
462	468		// reported immediately instead of after the buffered bytes are used.

@@ -750,19 +756,12 @@ func (r *Registry) handleGlobal(data []byte) {
750	756
751	757		// Call specific handler if registered
752	758		r.mu.RLock()
753		-	handler, specific := r.handlers[iface]
754		-	wildcard, all := r.handlers["*"]
	759	+	handlers := append([]GlobalHandler(nil), r.handlers[iface]...)
	760	+	handlers = append(handlers, r.handlers["*"]...)
755	761		r.mu.RUnlock()
756		-	if specific {
757		-
	762	+	for _, handler := range handlers {
758	763		handler(r, name, version)
759	764		}
760		-
761		-	// Call wildcard handler if registered
762		-	if all {
763		-
764		-	wildcard(r, name, version)
765		-	}
766	765		}
767	766
768	767		// handleGlobalRemove handles global removal

@@ -782,11 +781,16 @@ func (r *Registry) handleGlobalRemove(data []byte) {
782	781		}
783	782		}
784	783
785		-	// AddHandler adds a handler for a specific interface
	784	+	// AddHandler adds a handler for a specific interface, or "*" for every
	785	+	// interface. Handlers accumulate; each announced global calls all of them in
	786	+	// registration order.
786	787		func (r *Registry) AddHandler(iface string, handler GlobalHandler) {
	788	+	if handler == nil {
	789	+	return
	790	+	}
787	791		r.mu.Lock()
788	792		defer r.mu.Unlock()
789		-	r.handlers[iface] = handler
	793	+	r.handlers[iface] = append(r.handlers[iface], handler)
790	794		}
791	795
792	796		// ErrGlobalNotFound reports that the compositor does not announce a global.

 		id:       d.allocateID(),
 		display:  d,
 		globals:  make(map[uint32]Global),
-		handlers: make(map[string]GlobalHandler),
+		handlers: make(map[string][]GlobalHandler),
 	}
 	d.objects.Store(d.registry.id, d.registry)
 	return d
 	display        *Display
 	globals        map[uint32]Global
 	mu             sync.RWMutex
-	handlers       map[string]GlobalHandler
+	handlers       map[string][]GlobalHandler
 	removeHandlers []RegistryGlobalRemoveHandler
 }
 	case nil:
 		// Null object
 		return binary.Write(buf, binary.LittleEndian, uint32(0))
-	case int:
-		// File descriptor - write placeholder in message
-		// Actual FD will be sent via SCM_RIGHTS
-		return binary.Write(buf, binary.LittleEndian, uint32(0))
 	case uintptr:
 		// File descriptor as uintptr - NO placeholder in message for neurlang compatibility
 		// FD is ONLY sent via SCM_RIGHTS, not in the message body
 // Read boundaries never define message boundaries: a header split across
 // several reads and several messages arriving in one read are both handled.
 // Dispatch returns net.ErrClosed after Close, a typed ProtocolError for a
-// malformed frame, and io.ErrUnexpectedEOF when the peer closes mid-frame.
+// malformed frame, io.ErrUnexpectedEOF when the peer closes mid-frame, and
+// os.ErrDeadlineExceeded (not sticky) when a read deadline expires.
+//
+// Event handlers run while Dispatch holds the dispatch lock, so a handler must
+// not call Dispatch or Roundtrip on the same Display: that call deadlocks.
+// Hand work that needs a roundtrip to another goroutine instead.
 func (d *Display) Dispatch() error {
 	d.dispatchMu.Lock()
 	defer d.dispatchMu.Unlock()
 		if errors.Is(err, io.EOF) {
 			err = io.ErrUnexpectedEOF
 		}
+		// A missed deadline is not a broken stream: report it without making
+		// it sticky, so a later Dispatch can read again.
+		if errors.Is(err, os.ErrDeadlineExceeded) {
+			return err
+		}
 		d.readErr = err
 		// A control-message failure corrupts the descriptor stream, so it is
 		// reported immediately instead of after the buffered bytes are used.
 	// Call specific handler if registered
 	r.mu.RLock()
-	handler, specific := r.handlers[iface]
+	handlers := append([]GlobalHandler(nil), r.handlers[iface]...)
-	wildcard, all := r.handlers["*"]
+	handlers = append(handlers, r.handlers["*"]...)
 	r.mu.RUnlock()
-	if specific {
+	for _, handler := range handlers {
 		handler(r, name, version)
 	}
-	// Call wildcard handler if registered
-	if all {
-		wildcard(r, name, version)
-	}
 }
 // handleGlobalRemove handles global removal
 	}
 }
-// AddHandler adds a handler for a specific interface
+// AddHandler adds a handler for a specific interface, or "*" for every
+// interface. Handlers accumulate; each announced global calls all of them in
+// registration order.
 func (r *Registry) AddHandler(iface string, handler GlobalHandler) {
+	if handler == nil {
+		return
+	}
 	r.mu.Lock()
 	defer r.mu.Unlock()
-	r.handlers[iface] = handler
+	r.handlers[iface] = append(r.handlers[iface], handler)
 }
 // ErrGlobalNotFound reports that the compositor does not announce a global.

A hardening_test.go +68 -0

@@ -0,0 +1,68 @@
	1	+	package wlturbo
	2	+
	3	+	import (
	4	+	"bytes"
	5	+	"errors"
	6	+	"net"
	7	+	"os"
	8	+	"testing"
	9	+	"time"
	10	+	)
	11	+
	12	+	// A read deadline must not leave the Display permanently broken.
	13	+	func TestDispatchDeadlineIsNotSticky(t *testing.T) {
	14	+	client, server := net.Pipe()
	15	+	defer client.Close()
	16	+	defer server.Close()
	17	+	d := newDisplay(client)
	18	+	records := []EventRecord{}
	19	+	p := &recordingProxy{BaseProxy: BaseProxy{id: 7, context: d.context}, records: &records}
	20	+	d.objects.Store(uint32(7), p)
	21	+	d.RegisterEventSignature(7, 4, "uint,")
	22	+
	23	+	if err := client.SetReadDeadline(time.Now().Add(20 * time.Millisecond)); err != nil {
	24	+	t.Fatal(err)
	25	+	}
	26	+	if err := d.Dispatch(); !errors.Is(err, os.ErrDeadlineExceeded) {
	27	+	t.Fatalf("Dispatch = %v, want deadline exceeded", err)
	28	+	}
	29	+	if err := client.SetReadDeadline(time.Time{}); err != nil {
	30	+	t.Fatal(err)
	31	+	}
	32	+	go func() { _, _ = server.Write(message(7, 4, []byte{1, 0, 0, 0})) }()
	33	+	if err := d.Dispatch(); err != nil {
	34	+	t.Fatalf("Dispatch after clearing deadline = %v", err)
	35	+	}
	36	+	if len(records) != 1 {
	37	+	t.Fatalf("records = %d, want 1", len(records))
	38	+	}
	39	+	}
	40	+
	41	+	type countingGlobalHandler struct{ n *int }
	42	+
	43	+	func (h countingGlobalHandler) HandleRegistryGlobal(RegistryGlobalEvent) { *h.n++ }
	44	+
	45	+	// Every registered global handler runs, not only the last one.
	46	+	func TestRegistryGlobalHandlersAccumulate(t *testing.T) {
	47	+	d := newDisplay(&chunkConn{})
	48	+	var a, b, specific int
	49	+	d.registry.AddGlobalHandler(countingGlobalHandler{&a})
	50	+	d.registry.AddGlobalHandler(countingGlobalHandler{&b})
	51	+	d.registry.AddHandler("abc", func(*Registry, uint32, uint32) { specific++ })
	52	+	d.registry.handleGlobal([]byte{1, 0, 0, 0, 4, 0, 0, 0, 'a', 'b', 'c', 0, 1, 0, 0, 0})
	53	+	if a != 1 \|\| b != 1 \|\| specific != 1 {
	54	+	t.Fatalf("calls a=%d b=%d specific=%d, want 1 each", a, b, specific)
	55	+	}
	56	+	}
	57	+
	58	+	// A plain Go int is ambiguous (value or descriptor) and must be rejected
	59	+	// instead of being written as zero.
	60	+	func TestMarshalRejectsPlainInt(t *testing.T) {
	61	+	d := newDisplay(&chunkConn{})
	62	+	if err := d.marshalArg(&bytes.Buffer{}, 42); err == nil {
	63	+	t.Fatal("marshalArg(int) succeeded, want error")
	64	+	}
	65	+	if err := d.SendRequest(9, 0, 42); err == nil {
	66	+	t.Fatal("SendRequest with int argument succeeded, want error")
	67	+	}
	68	+	}

@@ -0,0 +1,68 @@
	1	+	package wlturbo
	2	+
	3	+	import (
	4	+	"bytes"
	5	+	"errors"
	6	+	"net"
	7	+	"os"
	8	+	"testing"
	9	+	"time"
	10	+	)
	11	+
	12	+	// A read deadline must not leave the Display permanently broken.
	13	+	func TestDispatchDeadlineIsNotSticky(t *testing.T) {
	14	+	client, server := net.Pipe()
	15	+	defer client.Close()
	16	+	defer server.Close()
	17	+	d := newDisplay(client)
	18	+	records := []EventRecord{}
	19	+	p := &recordingProxy{BaseProxy: BaseProxy{id: 7, context: d.context}, records: &records}
	20	+	d.objects.Store(uint32(7), p)
	21	+	d.RegisterEventSignature(7, 4, "uint,")
	22	+
	23	+	if err := client.SetReadDeadline(time.Now().Add(20 * time.Millisecond)); err != nil {
	24	+	t.Fatal(err)
	25	+	}
	26	+	if err := d.Dispatch(); !errors.Is(err, os.ErrDeadlineExceeded) {
	27	+	t.Fatalf("Dispatch = %v, want deadline exceeded", err)
	28	+	}
	29	+	if err := client.SetReadDeadline(time.Time{}); err != nil {
	30	+	t.Fatal(err)
	31	+	}
	32	+	go func() { _, _ = server.Write(message(7, 4, []byte{1, 0, 0, 0})) }()
	33	+	if err := d.Dispatch(); err != nil {
	34	+	t.Fatalf("Dispatch after clearing deadline = %v", err)
	35	+	}
	36	+	if len(records) != 1 {
	37	+	t.Fatalf("records = %d, want 1", len(records))
	38	+	}
	39	+	}
	40	+
	41	+	type countingGlobalHandler struct{ n *int }
	42	+
	43	+	func (h countingGlobalHandler) HandleRegistryGlobal(RegistryGlobalEvent) { *h.n++ }
	44	+
	45	+	// Every registered global handler runs, not only the last one.
	46	+	func TestRegistryGlobalHandlersAccumulate(t *testing.T) {
	47	+	d := newDisplay(&chunkConn{})
	48	+	var a, b, specific int
	49	+	d.registry.AddGlobalHandler(countingGlobalHandler{&a})
	50	+	d.registry.AddGlobalHandler(countingGlobalHandler{&b})
	51	+	d.registry.AddHandler("abc", func(*Registry, uint32, uint32) { specific++ })
	52	+	d.registry.handleGlobal([]byte{1, 0, 0, 0, 4, 0, 0, 0, 'a', 'b', 'c', 0, 1, 0, 0, 0})
	53	+	if a != 1 \|\| b != 1 \|\| specific != 1 {
	54	+	t.Fatalf("calls a=%d b=%d specific=%d, want 1 each", a, b, specific)
	55	+	}
	56	+	}
	57	+
	58	+	// A plain Go int is ambiguous (value or descriptor) and must be rejected
	59	+	// instead of being written as zero.
	60	+	func TestMarshalRejectsPlainInt(t *testing.T) {
	61	+	d := newDisplay(&chunkConn{})
	62	+	if err := d.marshalArg(&bytes.Buffer{}, 42); err == nil {
	63	+	t.Fatal("marshalArg(int) succeeded, want error")
	64	+	}
	65	+	if err := d.SendRequest(9, 0, 42); err == nil {
	66	+	t.Fatal("SendRequest with int argument succeeded, want error")
	67	+	}
	68	+	}