01f9a3d fix(xwm): check root ownership before claiming WM state and never adopt the check window · neferwl

M internal/adapters/wayland/xwm/events.go +2 -2


@@ -132,7 +132,7 @@ func (x XWM) onError(e xturbo.XError) {
132	132		func (x XWM) onCreateNotify(ev xturbo.Event) {
133	133		e := xproto.CreateNotifyOf(ev)
134	134		xid := uint32(e.Window())
135		-	if x.tab.lookup(xid) != 0 {
	135	+	if xid == x.wmWin \|\| x.tab.lookup(xid) != 0 {
136	136		return
137	137		}
138	138		s := x.insert(xid)

@@ -184,7 +184,7 @@ func (x XWM) onTreeReply(r xturbo.Reply) {
184	184		children := q.Children()
185	185		for i := 0; i < children.Len(); i++ {
186	186		xid := uint32(children.At(i))
187		-	if x.tab.lookup(xid) != 0 {
	187	+	if xid == x.wmWin \|\| x.tab.lookup(xid) != 0 {
188	188		continue
189	189		}
190	190		s := x.insert(xid)


@@ -132,7 +132,7 @@ func (x XWM) onError(e xturbo.XError) {
132		func (x XWM) onCreateNotify(ev xturbo.Event) {	132		func (x XWM) onCreateNotify(ev xturbo.Event) {
133		e := xproto.CreateNotifyOf(ev)	133		e := xproto.CreateNotifyOf(ev)
134		xid := uint32(e.Window())	134		xid := uint32(e.Window())
135	-	if x.tab.lookup(xid) != 0 {	135	+	if xid == x.wmWin \|\| x.tab.lookup(xid) != 0 {
136		return	136		return
137		}	137		}
138		s := x.insert(xid)	138		s := x.insert(xid)

@@ -184,7 +184,7 @@ func (x XWM) onTreeReply(r xturbo.Reply) {
184		children := q.Children()	184		children := q.Children()
185		for i := 0; i < children.Len(); i++ {	185		for i := 0; i < children.Len(); i++ {
186		xid := uint32(children.At(i))	186		xid := uint32(children.At(i))
187	-	if x.tab.lookup(xid) != 0 {	187	+	if xid == x.wmWin \|\| x.tab.lookup(xid) != 0 {
188		continue	188		continue
189		}	189		}
190		s := x.insert(xid)	190		s := x.insert(xid)

M internal/adapters/wayland/xwm/xwm.go +20 -2


@@ -13,6 +13,7 @@ import (
13	13		"encoding/binary"
14	14		"errors"
15	15		"fmt"
	16	+	"io"
16	17
17	18		"bnema.dev/xturbo"
18	19		"bnema.dev/xturbo/composite"

@@ -80,7 +81,7 @@ type XWM struct {
80	81		}
81	82
82	83		func newXWM(conn xturbo.Conn, log zerowrap.Logger) XWM {
83		-	x := &XWM{conn: conn, log: log, root: conn.Root(), tab: newTable(initialSlots)}
	84	+	x := &XWM{conn: conn, log: log.WithField("component", "xwm"), root: conn.Root(), tab: newTable(initialSlots)}
84	85		x.h = xturbo.Handler{OnEvent: x.onEvent, OnReply: x.onReply, OnError: x.onError}
85	86		x.fit()
86	87		return x

@@ -195,6 +196,17 @@ func (x *XWM) boot() error {
195	196		var rootMask xproto.ChangeWindowAttributesValues
196	197		rootMask.SetEventMask(xproto.EventMaskSubstructureRedirect \| xproto.EventMaskSubstructureNotify \| xproto.EventMaskPropertyChange)
197	198		x.expect(xproto.ChangeWindowAttributes(b, root, &rootMask), kindRootMask, 0, 0)
	199	+	// Only one client may select SubstructureRedirect on the root: wait for
	200	+	// the answer before touching anything another window manager owns.
	201	+	if err := b.Err(); err != nil {
	202	+	return fmt.Errorf("xwm: encode root mask: %w", err)
	203	+	}
	204	+	if err := conn.Sync(&x.h); err != nil {
	205	+	return fmt.Errorf("xwm: root mask: %w", err)
	206	+	}
	207	+	if x.bootErr != nil {
	208	+	return x.bootErr
	209	+	}
198	210		composite.RedirectSubwindows(b, root, composite.RedirectManual)
199	211
200	212		var wv xproto.CreateWindowValues

@@ -205,6 +217,8 @@ func (x *XWM) boot() error {
205	217		xproto.Atom(x.atoms[atomUTF8String]), 8, uint32(len(name)), name)
206	218		x.setProp32(x.root, atomNetSupportingWMCheck, xproto.AtomWindow, []uint32{x.wmWin})
207	219		x.setProp32(x.wmWin, atomNetSupportingWMCheck, xproto.AtomWindow, []uint32{x.wmWin})
	220	+	// CurrentTime and no MANAGER announcement: this Xwayland is private to
	221	+	// neferwl and nobody waits on these selections.
208	222		xproto.SetSelectionOwner(b, xproto.Window(x.wmWin), xproto.Atom(x.atoms[atomWMS0]), 0)
209	223		xproto.SetSelectionOwner(b, xproto.Window(x.wmWin), xproto.Atom(x.atoms[atomNetWMCMS0]), 0)
210	224

@@ -311,7 +325,11 @@ func (x *XWM) fail(err error) {
311	325		if x.closed {
312	326		return
313	327		}
314		-	x.log.Warn().Err(err).Msg("X11 window management stopped")
	328	+	if errors.Is(err, io.EOF) \|\| errors.Is(err, xturbo.ErrClosed) {
	329	+	x.log.Info().Err(err).Msg("X11 window management stopped: Xwayland closed the connection")
	330	+	} else {
	331	+	x.log.Warn().Err(err).Msg("X11 window management stopped")
	332	+	}
315	333		x.Close()
316	334		}
317	335

 	"encoding/binary"
 	"errors"
 	"fmt"
+	"io"
 	"bnema.dev/xturbo"
 	"bnema.dev/xturbo/composite"
 }
 func newXWM(conn *xturbo.Conn, log zerowrap.Logger) *XWM {
-	x := &XWM{conn: conn, log: log, root: conn.Root(), tab: newTable(initialSlots)}
+	x := &XWM{conn: conn, log: log.WithField("component", "xwm"), root: conn.Root(), tab: newTable(initialSlots)}
 	x.h = xturbo.Handler{OnEvent: x.onEvent, OnReply: x.onReply, OnError: x.onError}
 	x.fit()
 	return x
 	var rootMask xproto.ChangeWindowAttributesValues
 	rootMask.SetEventMask(xproto.EventMaskSubstructureRedirect | xproto.EventMaskSubstructureNotify | xproto.EventMaskPropertyChange)
 	x.expect(xproto.ChangeWindowAttributes(b, root, &rootMask), kindRootMask, 0, 0)
+	// Only one client may select SubstructureRedirect on the root: wait for
+	// the answer before touching anything another window manager owns.
+	if err := b.Err(); err != nil {
+		return fmt.Errorf("xwm: encode root mask: %w", err)
+	}
+	if err := conn.Sync(&x.h); err != nil {
+		return fmt.Errorf("xwm: root mask: %w", err)
+	}
+	if x.bootErr != nil {
+		return x.bootErr
+	}
 	composite.RedirectSubwindows(b, root, composite.RedirectManual)
 	var wv xproto.CreateWindowValues
 		xproto.Atom(x.atoms[atomUTF8String]), 8, uint32(len(name)), name)
 	x.setProp32(x.root, atomNetSupportingWMCheck, xproto.AtomWindow, []uint32{x.wmWin})
 	x.setProp32(x.wmWin, atomNetSupportingWMCheck, xproto.AtomWindow, []uint32{x.wmWin})
+	// CurrentTime and no MANAGER announcement: this Xwayland is private to
+	// neferwl and nobody waits on these selections.
 	xproto.SetSelectionOwner(b, xproto.Window(x.wmWin), xproto.Atom(x.atoms[atomWMS0]), 0)
 	xproto.SetSelectionOwner(b, xproto.Window(x.wmWin), xproto.Atom(x.atoms[atomNetWMCMS0]), 0)
 	if x.closed {
 		return
 	}
-	x.log.Warn().Err(err).Msg("X11 window management stopped")
+	if errors.Is(err, io.EOF) || errors.Is(err, xturbo.ErrClosed) {
+		x.log.Info().Err(err).Msg("X11 window management stopped: Xwayland closed the connection")
+	} else {
+		x.log.Warn().Err(err).Msg("X11 window management stopped")
+	}
 	x.Close()
 }

M internal/adapters/wayland/xwm/xwm_test.go +18 -0


@@ -137,6 +137,13 @@ func TestBootWMExists(t *testing.T) {
137	137		x, err := Boot(fd, testLog())
138	138		require.ErrorIs(t, err, ErrWMExists)
139	139		require.Nil(t, x)
	140	+	// Nothing the running window manager owns was touched.
	141	+	for _, r := range p.requests() {
	142	+	require.NotEqual(t, uint8(opSetSelectionOwner), r.major)
	143	+	if r.major == opChangeProperty {
	144	+	require.NotEqual(t, uint32(peerRoot), r.u32(4))
	145	+	}
	146	+	}
140	147		}
141	148
142	149		func TestBootNoComposite(t *testing.T) {

@@ -238,6 +245,17 @@ func TestCreateAndDestroyManaged(t *testing.T) {
238	245		_ = src
239	246		}
240	247
	248	+	// The X server reports the XWM's own check window to it like any other
	249	+	// child of the root; it is never adopted.
	250	+	func TestOwnWindowNeverAdopted(t *testing.T) {
	251	+	x, p, _, _, _ := attached(t) // the mock fails the test on any host call
	252	+	p.createNotify(x.wmWin, false)
	253	+	const marker = 0x500003
	254	+	p.createNotify(marker, true)
	255	+	readUntil(t, x, func() bool { return x.Slot(marker) != 0 })
	256	+	require.Zero(t, x.Slot(x.wmWin))
	257	+	}
	258	+
241	259		func TestOverrideRedirectNeverReachesHost(t *testing.T) {
242	260		x, p, _, _, _ := attached(t) // the mock fails the test on any host call
243	261		p.createNotify(0x500002, true)


@@ -137,6 +137,13 @@ func TestBootWMExists(t *testing.T) {
137	x, err := Boot(fd, testLog())	137		x, err := Boot(fd, testLog())
138	require.ErrorIs(t, err, ErrWMExists)	138		require.ErrorIs(t, err, ErrWMExists)
139	require.Nil(t, x)	139		require.Nil(t, x)
		140	+	// Nothing the running window manager owns was touched.
		141	+	for _, r := range p.requests() {
		142	+	require.NotEqual(t, uint8(opSetSelectionOwner), r.major)
		143	+	if r.major == opChangeProperty {
		144	+	require.NotEqual(t, uint32(peerRoot), r.u32(4))
		145	+	}
		146	+	}
140	}	147		}
141		148
142	func TestBootNoComposite(t *testing.T) {	149		func TestBootNoComposite(t *testing.T) {

@@ -238,6 +245,17 @@ func TestCreateAndDestroyManaged(t *testing.T) {
238	_ = src	245		_ = src
239	}	246		}
240		247
		248	+	// The X server reports the XWM's own check window to it like any other
		249	+	// child of the root; it is never adopted.
		250	+	func TestOwnWindowNeverAdopted(t *testing.T) {
		251	+	x, p, _, _, _ := attached(t) // the mock fails the test on any host call
		252	+	p.createNotify(x.wmWin, false)
		253	+	const marker = 0x500003
		254	+	p.createNotify(marker, true)
		255	+	readUntil(t, x, func() bool { return x.Slot(marker) != 0 })
		256	+	require.Zero(t, x.Slot(x.wmWin))
		257	+	}
		258	+
241	func TestOverrideRedirectNeverReachesHost(t *testing.T) {	259		func TestOverrideRedirectNeverReachesHost(t *testing.T) {
242	x, p, _, _, _ := attached(t) // the mock fails the test on any host call	260		x, p, _, _, _ := attached(t) // the mock fails the test on any host call
243	p.createNotify(0x500002, true)	261		p.createNotify(0x500002, true)