fix(wayland): read placement tokens independently of command drain
Read the process environment only on the first toplevel map, never on ordinary commits or remaps. Pending tokens remain validated by core. Keep the advisory pending command and existing security delivery barrier without relying on display application order.
- commit
- d2670da939565947bb3c3a1de89353d492604273
- author
- brice · · 2026-10-19 18:48 UTC
- parent
- 353b4e3
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +22 -21
M internal/adapters/wayland/slot.go +4 -5
| @@ -37,12 +37,11 @@ func (linuxProcEnv) Lookup(pid int, key string) (string, bool) { | |||
| 37 | 37 | } | |
| 38 | 38 | ||
| 39 | 39 | // slotToken returns the SlotEnv value of the client process, if any. It | |
| 40 | - | // reads /proc only while core waits for a slot window. There is no per-client | |
| 41 | - | // cache: libwayland may reuse a client address after a disconnect. | |
| 40 | + | // reads /proc once at toplevel mapping, independently of command application: | |
| 41 | + | // a spawned client can map before the display drains core's commands. Core | |
| 42 | + | // accepts only tokens for pending placements. There is no per-client cache: | |
| 43 | + | // libwayland may reuse a client address after a disconnect. | |
| 42 | 44 | func (s *Server) slotToken(c server.Client) string { | |
| 43 | - | if !s.slotsPending { | |
| 44 | - | return "" | |
| 45 | - | } | |
| 46 | 45 | v, _ := s.env.Lookup(c.PID(), ports.SlotEnv) | |
| 47 | 46 | return v | |
| 48 | 47 | } | |
| @@ -37,12 +37,11 @@ func (linuxProcEnv) Lookup(pid int, key string) (string, bool) { | |||||
| 37 | } | 37 | } | ||
| 38 | 38 | ||||
| 39 | // slotToken returns the SlotEnv value of the client process, if any. It | 39 | // slotToken returns the SlotEnv value of the client process, if any. It | ||
| 40 | - | // reads /proc only while core waits for a slot window. There is no per-client | 40 | + | // reads /proc once at toplevel mapping, independently of command application: |
| 41 | - | // cache: libwayland may reuse a client address after a disconnect. | 41 | + | // a spawned client can map before the display drains core's commands. Core |
| 42 | + | // accepts only tokens for pending placements. There is no per-client cache: | |||
| 43 | + | // libwayland may reuse a client address after a disconnect. | |||
| 42 | func (s *Server) slotToken(c server.Client) string { | 44 | func (s *Server) slotToken(c server.Client) string { | ||
| 43 | - | if !s.slotsPending { | |||
| 44 | - | return "" | |||
| 45 | - | } | |||
| 46 | v, _ := s.env.Lookup(c.PID(), ports.SlotEnv) | 45 | v, _ := s.env.Lookup(c.PID(), ports.SlotEnv) | ||
| 47 | return v | 46 | return v | ||
| 48 | } | 47 | } | ||
M internal/adapters/wayland/slot_test.go +7 -9
| @@ -14,17 +14,15 @@ func TestWindowMappedCarriesSlot(t *testing.T) { | |||
| 14 | 14 | env.EXPECT().Lookup(os.Getpid(), ports.SlotEnv).Return("7", true).Once() | |
| 15 | 15 | s, events, _, dir := keyboardServer(t, func(s *Server) { s.env = env }) | |
| 16 | 16 | mapWindow := toplevelMapper(t, protocolClient(t, s, dir), events) | |
| 17 | - | // No slot pending: /proc is not read (the mock would fail on a call). | |
| 18 | - | if w := mapWindow(); w.Slot != "" || w.PID != os.Getpid() { | |
| 17 | + | // Hold the pending command outside the display until after mapping. The | |
| 18 | + | // protocol connection must carry the token even when command application | |
| 19 | + | // loses the race to this fast client's first window. | |
| 20 | + | pending := ports.SlotsPending{Pending: true} | |
| 21 | + | if w := mapWindow(); w.Slot != "7" || w.PID != os.Getpid() { | |
| 19 | 22 | t.Fatalf("slot %q pid %d", w.Slot, w.PID) | |
| 20 | 23 | } | |
| 21 | - | // This tests environment lookup, not command transport. Apply the gate | |
| 22 | - | // synchronously before mapping over the independent protocol connection. | |
| 23 | - | if !s.display.Do(func() { s.apply(ports.SlotsPending{Pending: true}) }) { | |
| 24 | - | t.Fatal("display stopped before applying SlotsPending") | |
| 25 | - | } | |
| 26 | - | if w := mapWindow(); w.Slot != "7" { | |
| 27 | - | t.Fatalf("slot %q", w.Slot) | |
| 24 | + | if !s.display.Do(func() { s.apply(pending) }) { | |
| 25 | + | t.Fatal("display stopped before applying delayed command") | |
| 28 | 26 | } | |
| 29 | 27 | } | |
| 30 | 28 | ||
| @@ -14,17 +14,15 @@ func TestWindowMappedCarriesSlot(t *testing.T) { | |||||
| 14 | env.EXPECT().Lookup(os.Getpid(), ports.SlotEnv).Return("7", true).Once() | 14 | env.EXPECT().Lookup(os.Getpid(), ports.SlotEnv).Return("7", true).Once() | ||
| 15 | s, events, _, dir := keyboardServer(t, func(s *Server) { s.env = env }) | 15 | s, events, _, dir := keyboardServer(t, func(s *Server) { s.env = env }) | ||
| 16 | mapWindow := toplevelMapper(t, protocolClient(t, s, dir), events) | 16 | mapWindow := toplevelMapper(t, protocolClient(t, s, dir), events) | ||
| 17 | - | // No slot pending: /proc is not read (the mock would fail on a call). | 17 | + | // Hold the pending command outside the display until after mapping. The |
| 18 | - | if w := mapWindow(); w.Slot != "" || w.PID != os.Getpid() { | 18 | + | // protocol connection must carry the token even when command application |
| 19 | + | // loses the race to this fast client's first window. | |||
| 20 | + | pending := ports.SlotsPending{Pending: true} | |||
| 21 | + | if w := mapWindow(); w.Slot != "7" || w.PID != os.Getpid() { | |||
| 19 | t.Fatalf("slot %q pid %d", w.Slot, w.PID) | 22 | t.Fatalf("slot %q pid %d", w.Slot, w.PID) | ||
| 20 | } | 23 | } | ||
| 21 | - | // This tests environment lookup, not command transport. Apply the gate | 24 | + | if !s.display.Do(func() { s.apply(pending) }) { |
| 22 | - | // synchronously before mapping over the independent protocol connection. | 25 | + | t.Fatal("display stopped before applying delayed command") |
| 23 | - | if !s.display.Do(func() { s.apply(ports.SlotsPending{Pending: true}) }) { | |||
| 24 | - | t.Fatal("display stopped before applying SlotsPending") | |||
| 25 | - | } | |||
| 26 | - | if w := mapWindow(); w.Slot != "7" { | |||
| 27 | - | t.Fatalf("slot %q", w.Slot) | |||
| 28 | } | 26 | } | ||
| 29 | } | 27 | } | ||
| 30 | 28 | ||||
M internal/adapters/wayland/xdg.go +6 -1
| @@ -239,7 +239,12 @@ func (x *xdgSurface) GetToplevel(r *xdgshell.Surface, id uint32) { | |||
| 239 | 239 | } else if buffer && !w.mapped && x.acked { | |
| 240 | 240 | w.mapped = true | |
| 241 | 241 | w.gen++ | |
| 242 | - | slot := x.server.slotToken(r.Client()) | |
| 242 | + | // Placement claims are for a new toplevel, not later remaps. | |
| 243 | + | // Ordinary commits, popups and remaps need no /proc lookup. | |
| 244 | + | var slot string | |
| 245 | + | if w.gen == 1 { | |
| 246 | + | slot = x.server.slotToken(r.Client()) | |
| 247 | + | } | |
| 243 | 248 | // Its size comes with the commit, in WindowResized (afterCommit). | |
| 244 | 249 | w.floating = w.floats() | |
| 245 | 250 | var parent ports.WindowID | |
| @@ -239,7 +239,12 @@ func (x *xdgSurface) GetToplevel(r *xdgshell.Surface, id uint32) { | |||||
| 239 | } else if buffer && !w.mapped && x.acked { | 239 | } else if buffer && !w.mapped && x.acked { | ||
| 240 | w.mapped = true | 240 | w.mapped = true | ||
| 241 | w.gen++ | 241 | w.gen++ | ||
| 242 | - | slot := x.server.slotToken(r.Client()) | 242 | + | // Placement claims are for a new toplevel, not later remaps. |
| 243 | + | // Ordinary commits, popups and remaps need no /proc lookup. | |||
| 244 | + | var slot string | |||
| 245 | + | if w.gen == 1 { | |||
| 246 | + | slot = x.server.slotToken(r.Client()) | |||
| 247 | + | } | |||
| 243 | // Its size comes with the commit, in WindowResized (afterCommit). | 248 | // Its size comes with the commit, in WindowResized (afterCommit). | ||
| 244 | w.floating = w.floats() | 249 | w.floating = w.floats() | ||
| 245 | var parent ports.WindowID | 250 | var parent ports.WindowID | ||
M internal/core/core.go +3 -4
| @@ -473,10 +473,9 @@ func (c *Core) command(ctx context.Context, v ports.ClientCommand) error { | |||
| 473 | 473 | return err | |
| 474 | 474 | } | |
| 475 | 475 | ||
| 476 | - | // commandFlushed is command for a command that another channel depends on: | |
| 477 | - | // it returns once v is in the command channel. SlotsPending must reach | |
| 478 | - | // wayland before the spawn whose window it lets wayland claim; the spawn | |
| 479 | - | // goes on its own channel, so queue order alone does not keep them in order. | |
| 476 | + | // commandFlushed returns once v is in the command channel, not once the | |
| 477 | + | // display has applied it. Pending publication retains this delivery barrier | |
| 478 | + | // and its security-epoch recheck, but token lookup must not depend on it. | |
| 480 | 479 | func (c *Core) commandFlushed(ctx context.Context, v ports.ClientCommand) error { | |
| 481 | 480 | seq, err := c.queue(ctx, v) | |
| 482 | 481 | if err != nil || seq == 0 { | |
| @@ -473,10 +473,9 @@ func (c *Core) command(ctx context.Context, v ports.ClientCommand) error { | |||||
| 473 | return err | 473 | return err | ||
| 474 | } | 474 | } | ||
| 475 | 475 | ||||
| 476 | - | // commandFlushed is command for a command that another channel depends on: | 476 | + | // commandFlushed returns once v is in the command channel, not once the |
| 477 | - | // it returns once v is in the command channel. SlotsPending must reach | 477 | + | // display has applied it. Pending publication retains this delivery barrier |
| 478 | - | // wayland before the spawn whose window it lets wayland claim; the spawn | 478 | + | // and its security-epoch recheck, but token lookup must not depend on it. |
| 479 | - | // goes on its own channel, so queue order alone does not keep them in order. | |||
| 480 | func (c *Core) commandFlushed(ctx context.Context, v ports.ClientCommand) error { | 479 | func (c *Core) commandFlushed(ctx context.Context, v ports.ClientCommand) error { | ||
| 481 | seq, err := c.queue(ctx, v) | 480 | seq, err := c.queue(ctx, v) | ||
| 482 | if err != nil || seq == 0 { | 481 | if err != nil || seq == 0 { | ||
M internal/ports/client_commands.go +2 -2
| @@ -120,8 +120,8 @@ type SetOutputs struct { | |||
| 120 | 120 | ||
| 121 | 121 | func (SetOutputs) clientCommand() {} | |
| 122 | 122 | ||
| 123 | - | // SlotsPending tells wayland whether a slot waits for its window. Only then | |
| 124 | - | // does it read the SlotEnv of mapping clients. | |
| 123 | + | // SlotsPending reports whether core awaits a placement window. It is advisory: | |
| 124 | + | // mapping clients' SlotEnv must be read independently of command application. | |
| 125 | 125 | type SlotsPending struct{ Pending bool } | |
| 126 | 126 | ||
| 127 | 127 | func (SlotsPending) clientCommand() {} | |
| @@ -120,8 +120,8 @@ type SetOutputs struct { | |||||
| 120 | 120 | ||||
| 121 | func (SetOutputs) clientCommand() {} | 121 | func (SetOutputs) clientCommand() {} | ||
| 122 | 122 | ||||
| 123 | - | // SlotsPending tells wayland whether a slot waits for its window. Only then | 123 | + | // SlotsPending reports whether core awaits a placement window. It is advisory: |
| 124 | - | // does it read the SlotEnv of mapping clients. | 124 | + | // mapping clients' SlotEnv must be read independently of command application. |
| 125 | type SlotsPending struct{ Pending bool } | 125 | type SlotsPending struct{ Pending bool } | ||
| 126 | 126 | ||||
| 127 | func (SlotsPending) clientCommand() {} | 127 | func (SlotsPending) clientCommand() {} | ||