fix(server): make events on destroyed resources safe no-ops
- commit
- 90ef96bfd0183a1fffc82ea51931ab8f50e78fe1
- author
- brice · · 2026-09-23 12:14 UTC
- parent
- adae91e
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +95 -7
M internal/e2e/feasibility_test.go +68 -4
| @@ -16,7 +16,7 @@ import ( | |||
| 16 | 16 | ||
| 17 | 17 | // startServer runs a display with a wl_compositor global on a private socket. | |
| 18 | 18 | // commits counts wl_surface.commit requests received by Go handlers. | |
| 19 | - | func startServer(t *testing.T) (socket string, d *server.Display, commits chan uint32) { | |
| 19 | + | func startServer(t *testing.T, captured ...chan *server.Resource) (socket string, d *server.Display, commits chan uint32) { | |
| 20 | 20 | t.Helper() | |
| 21 | 21 | d, err := server.NewDisplay() | |
| 22 | 22 | if err != nil { | |
| @@ -42,6 +42,9 @@ func startServer(t *testing.T) (socket string, d *server.Display, commits chan u | |||
| 42 | 42 | var serial uint32 | |
| 43 | 43 | surface := &surfaceHandler{commits: commits, serial: &serial, t: t} | |
| 44 | 44 | compositor := &compositorHandler{surface: surface, t: t} | |
| 45 | + | if len(captured) > 0 { | |
| 46 | + | compositor.captured = captured[0] | |
| 47 | + | } | |
| 45 | 48 | err = wayland.NewCompositorGlobal(d, 1, func(c server.Client, version, id uint32) { | |
| 46 | 49 | if _, err := wayland.NewCompositor(c, int32(version), id, compositor); err != nil { | |
| 47 | 50 | t.Error(err) | |
| @@ -153,6 +156,61 @@ func TestRequestEventRoundtripAndDisconnect(t *testing.T) { | |||
| 153 | 156 | } | |
| 154 | 157 | } | |
| 155 | 158 | ||
| 159 | + | // A resource retained by Go must be harmless after libwayland destroys its client. | |
| 160 | + | func TestPostEventAfterDisconnect(t *testing.T) { | |
| 161 | + | captured := make(chan *server.Resource, 1) | |
| 162 | + | socket, d, _ := startServer(t, captured) | |
| 163 | + | c, err := wlturbo.Connect(socket) | |
| 164 | + | if err != nil { | |
| 165 | + | t.Fatal(err) | |
| 166 | + | } | |
| 167 | + | if err := c.Roundtrip(); err != nil { | |
| 168 | + | t.Fatal(err) | |
| 169 | + | } | |
| 170 | + | // Create a surface retained by the server after the client disconnects. | |
| 171 | + | g, ok := c.Registry().FindGlobal("wl_compositor") | |
| 172 | + | if !ok { | |
| 173 | + | t.Fatal("wl_compositor not advertised") | |
| 174 | + | } | |
| 175 | + | comp, err := c.Registry().BindID(g.Name, g.Interface, 1) | |
| 176 | + | if err != nil { | |
| 177 | + | t.Fatal(err) | |
| 178 | + | } | |
| 179 | + | surf := c.AllocateID() | |
| 180 | + | must(t, c.SendRequest(comp, uint16(wayland.CompositorRequestCreateSurface), surf)) | |
| 181 | + | if err := c.Roundtrip(); err != nil { | |
| 182 | + | t.Fatal(err) | |
| 183 | + | } | |
| 184 | + | stale := <-captured | |
| 185 | + | _ = c.Close() | |
| 186 | + | deadline := time.Now().Add(2 * time.Second) | |
| 187 | + | for { | |
| 188 | + | var n int | |
| 189 | + | d.Do(func() { n = server.LiveResources() }) | |
| 190 | + | if n == 0 { | |
| 191 | + | break | |
| 192 | + | } | |
| 193 | + | if time.Now().After(deadline) { | |
| 194 | + | t.Fatalf("%d resources still alive", n) | |
| 195 | + | } | |
| 196 | + | time.Sleep(5 * time.Millisecond) | |
| 197 | + | } | |
| 198 | + | d.Do(func() { | |
| 199 | + | if stale == nil { | |
| 200 | + | t.Error("resource was not created") | |
| 201 | + | return | |
| 202 | + | } | |
| 203 | + | if stale.Alive() { | |
| 204 | + | t.Error("resource still alive") | |
| 205 | + | } | |
| 206 | + | stale.PostEvent(0) | |
| 207 | + | stale.PostError(0, "gone") | |
| 208 | + | if stale.ID() != 0 || stale.Version() != 0 || stale.Client() != (server.Client{}) { | |
| 209 | + | t.Error("destroyed resource returned nonzero metadata") | |
| 210 | + | } | |
| 211 | + | }) | |
| 212 | + | } | |
| 213 | + | ||
| 156 | 214 | // doneProxy records wl_callback.done. | |
| 157 | 215 | type doneProxy struct { | |
| 158 | 216 | wlturbo.BaseProxy | |
| @@ -173,13 +231,19 @@ func must(t *testing.T, err error) { | |||
| 173 | 231 | } | |
| 174 | 232 | ||
| 175 | 233 | type compositorHandler struct { | |
| 176 | - | surface *surfaceHandler | |
| 177 | - | t *testing.T | |
| 234 | + | surface *surfaceHandler | |
| 235 | + | t *testing.T | |
| 236 | + | captured chan *server.Resource | |
| 178 | 237 | } | |
| 179 | 238 | ||
| 180 | 239 | func (h *compositorHandler) CreateSurface(self *wayland.Compositor, id uint32) { | |
| 181 | - | if _, err := wayland.NewSurface(self.Client(), self.Version(), id, h.surface); err != nil { | |
| 240 | + | r, err := wayland.NewSurface(self.Client(), self.Version(), id, h.surface) | |
| 241 | + | if err != nil { | |
| 182 | 242 | h.t.Error(err) | |
| 243 | + | return | |
| 244 | + | } | |
| 245 | + | if h.captured != nil { | |
| 246 | + | h.captured <- r.Resource | |
| 183 | 247 | } | |
| 184 | 248 | } | |
| 185 | 249 | func (h *compositorHandler) CreateRegion(self *wayland.Compositor, id uint32) { | |
| @@ -16,7 +16,7 @@ import ( | |||||
| 16 | 16 | ||||
| 17 | // startServer runs a display with a wl_compositor global on a private socket. | 17 | // startServer runs a display with a wl_compositor global on a private socket. | ||
| 18 | // commits counts wl_surface.commit requests received by Go handlers. | 18 | // commits counts wl_surface.commit requests received by Go handlers. | ||
| 19 | - | func startServer(t *testing.T) (socket string, d *server.Display, commits chan uint32) { | 19 | + | func startServer(t *testing.T, captured ...chan *server.Resource) (socket string, d *server.Display, commits chan uint32) { |
| 20 | t.Helper() | 20 | t.Helper() | ||
| 21 | d, err := server.NewDisplay() | 21 | d, err := server.NewDisplay() | ||
| 22 | if err != nil { | 22 | if err != nil { | ||
| @@ -42,6 +42,9 @@ func startServer(t *testing.T) (socket string, d *server.Display, commits chan u | |||||
| 42 | var serial uint32 | 42 | var serial uint32 | ||
| 43 | surface := &surfaceHandler{commits: commits, serial: &serial, t: t} | 43 | surface := &surfaceHandler{commits: commits, serial: &serial, t: t} | ||
| 44 | compositor := &compositorHandler{surface: surface, t: t} | 44 | compositor := &compositorHandler{surface: surface, t: t} | ||
| 45 | + | if len(captured) > 0 { | |||
| 46 | + | compositor.captured = captured[0] | |||
| 47 | + | } | |||
| 45 | err = wayland.NewCompositorGlobal(d, 1, func(c server.Client, version, id uint32) { | 48 | err = wayland.NewCompositorGlobal(d, 1, func(c server.Client, version, id uint32) { | ||
| 46 | if _, err := wayland.NewCompositor(c, int32(version), id, compositor); err != nil { | 49 | if _, err := wayland.NewCompositor(c, int32(version), id, compositor); err != nil { | ||
| 47 | t.Error(err) | 50 | t.Error(err) | ||
| @@ -153,6 +156,61 @@ func TestRequestEventRoundtripAndDisconnect(t *testing.T) { | |||||
| 153 | } | 156 | } | ||
| 154 | } | 157 | } | ||
| 155 | 158 | ||||
| 159 | + | // A resource retained by Go must be harmless after libwayland destroys its client. | |||
| 160 | + | func TestPostEventAfterDisconnect(t *testing.T) { | |||
| 161 | + | captured := make(chan *server.Resource, 1) | |||
| 162 | + | socket, d, _ := startServer(t, captured) | |||
| 163 | + | c, err := wlturbo.Connect(socket) | |||
| 164 | + | if err != nil { | |||
| 165 | + | t.Fatal(err) | |||
| 166 | + | } | |||
| 167 | + | if err := c.Roundtrip(); err != nil { | |||
| 168 | + | t.Fatal(err) | |||
| 169 | + | } | |||
| 170 | + | // Create a surface retained by the server after the client disconnects. | |||
| 171 | + | g, ok := c.Registry().FindGlobal("wl_compositor") | |||
| 172 | + | if !ok { | |||
| 173 | + | t.Fatal("wl_compositor not advertised") | |||
| 174 | + | } | |||
| 175 | + | comp, err := c.Registry().BindID(g.Name, g.Interface, 1) | |||
| 176 | + | if err != nil { | |||
| 177 | + | t.Fatal(err) | |||
| 178 | + | } | |||
| 179 | + | surf := c.AllocateID() | |||
| 180 | + | must(t, c.SendRequest(comp, uint16(wayland.CompositorRequestCreateSurface), surf)) | |||
| 181 | + | if err := c.Roundtrip(); err != nil { | |||
| 182 | + | t.Fatal(err) | |||
| 183 | + | } | |||
| 184 | + | stale := <-captured | |||
| 185 | + | _ = c.Close() | |||
| 186 | + | deadline := time.Now().Add(2 * time.Second) | |||
| 187 | + | for { | |||
| 188 | + | var n int | |||
| 189 | + | d.Do(func() { n = server.LiveResources() }) | |||
| 190 | + | if n == 0 { | |||
| 191 | + | break | |||
| 192 | + | } | |||
| 193 | + | if time.Now().After(deadline) { | |||
| 194 | + | t.Fatalf("%d resources still alive", n) | |||
| 195 | + | } | |||
| 196 | + | time.Sleep(5 * time.Millisecond) | |||
| 197 | + | } | |||
| 198 | + | d.Do(func() { | |||
| 199 | + | if stale == nil { | |||
| 200 | + | t.Error("resource was not created") | |||
| 201 | + | return | |||
| 202 | + | } | |||
| 203 | + | if stale.Alive() { | |||
| 204 | + | t.Error("resource still alive") | |||
| 205 | + | } | |||
| 206 | + | stale.PostEvent(0) | |||
| 207 | + | stale.PostError(0, "gone") | |||
| 208 | + | if stale.ID() != 0 || stale.Version() != 0 || stale.Client() != (server.Client{}) { | |||
| 209 | + | t.Error("destroyed resource returned nonzero metadata") | |||
| 210 | + | } | |||
| 211 | + | }) | |||
| 212 | + | } | |||
| 213 | + | ||||
| 156 | // doneProxy records wl_callback.done. | 214 | // doneProxy records wl_callback.done. | ||
| 157 | type doneProxy struct { | 215 | type doneProxy struct { | ||
| 158 | wlturbo.BaseProxy | 216 | wlturbo.BaseProxy | ||
| @@ -173,13 +231,19 @@ func must(t *testing.T, err error) { | |||||
| 173 | } | 231 | } | ||
| 174 | 232 | ||||
| 175 | type compositorHandler struct { | 233 | type compositorHandler struct { | ||
| 176 | - | surface *surfaceHandler | 234 | + | surface *surfaceHandler |
| 177 | - | t *testing.T | 235 | + | t *testing.T |
| 236 | + | captured chan *server.Resource | |||
| 178 | } | 237 | } | ||
| 179 | 238 | ||||
| 180 | func (h *compositorHandler) CreateSurface(self *wayland.Compositor, id uint32) { | 239 | func (h *compositorHandler) CreateSurface(self *wayland.Compositor, id uint32) { | ||
| 181 | - | if _, err := wayland.NewSurface(self.Client(), self.Version(), id, h.surface); err != nil { | 240 | + | r, err := wayland.NewSurface(self.Client(), self.Version(), id, h.surface) |
| 241 | + | if err != nil { | |||
| 182 | h.t.Error(err) | 242 | h.t.Error(err) | ||
| 243 | + | return | |||
| 244 | + | } | |||
| 245 | + | if h.captured != nil { | |||
| 246 | + | h.captured <- r.Resource | |||
| 183 | } | 247 | } | ||
| 184 | } | 248 | } | ||
| 185 | func (h *compositorHandler) CreateRegion(self *wayland.Compositor, id uint32) { | 249 | func (h *compositorHandler) CreateRegion(self *wayland.Compositor, id uint32) { | ||
M server/display.go +27 -3
| @@ -120,9 +120,25 @@ func (c Client) CreateResource(iface *Interface, version int32, id uint32, h Han | |||
| 120 | 120 | return r, nil | |
| 121 | 121 | } | |
| 122 | 122 | ||
| 123 | - | func (r *Resource) ID() uint32 { return wlResourceGetID(r.c) } | |
| 124 | - | func (r *Resource) Version() int32 { return wlResourceGetVersion(r.c) } | |
| 125 | - | func (r *Resource) Client() Client { return Client{wlResourceGetClient(r.c)} } | |
| 123 | + | func (r *Resource) Alive() bool { return !r.gone } | |
| 124 | + | func (r *Resource) ID() uint32 { | |
| 125 | + | if r.gone { | |
| 126 | + | return 0 | |
| 127 | + | } | |
| 128 | + | return wlResourceGetID(r.c) | |
| 129 | + | } | |
| 130 | + | func (r *Resource) Version() int32 { | |
| 131 | + | if r.gone { | |
| 132 | + | return 0 | |
| 133 | + | } | |
| 134 | + | return wlResourceGetVersion(r.c) | |
| 135 | + | } | |
| 136 | + | func (r *Resource) Client() Client { | |
| 137 | + | if r.gone { | |
| 138 | + | return Client{} | |
| 139 | + | } | |
| 140 | + | return Client{wlResourceGetClient(r.c)} | |
| 141 | + | } | |
| 126 | 142 | func (r *Resource) Iface() *Interface { return r.iface } | |
| 127 | 143 | ||
| 128 | 144 | // Destroy frees the resource. It is a no-op once the resource is gone, so | |
| @@ -135,7 +151,11 @@ func (r *Resource) Destroy() { | |||
| 135 | 151 | ||
| 136 | 152 | // PostEvent sends an event. Arguments must match the event signature; build | |
| 137 | 153 | // string and array arguments with a Pinner and unpin it after this returns. | |
| 154 | + | // Calling it after destruction is safe and does nothing. | |
| 138 | 155 | func (r *Resource) PostEvent(opcode uint32, args ...Arg) { | |
| 156 | + | if r.gone { | |
| 157 | + | return | |
| 158 | + | } | |
| 139 | 159 | var p unsafe.Pointer | |
| 140 | 160 | if len(args) > 0 { | |
| 141 | 161 | p = unsafe.Pointer(&args[0]) | |
| @@ -150,7 +170,11 @@ func (r *Resource) PostEvent(opcode uint32, args ...Arg) { | |||
| 150 | 170 | } | |
| 151 | 171 | ||
| 152 | 172 | // PostError sends a protocol error and disconnects the client. | |
| 173 | + | // Calling it after destruction is safe and does nothing. | |
| 153 | 174 | func (r *Resource) PostError(code uint32, msg string) { | |
| 175 | + | if r.gone { | |
| 176 | + | return | |
| 177 | + | } | |
| 154 | 178 | // wl_resource_post_error takes a printf format; escape it and pass no | |
| 155 | 179 | // variadic arguments (purego clears AL, as SysV varargs require). | |
| 156 | 180 | b := append([]byte(strings.ReplaceAll(msg, "%", "%%")), 0) | |
| @@ -120,9 +120,25 @@ func (c Client) CreateResource(iface *Interface, version int32, id uint32, h Han | |||||
| 120 | return r, nil | 120 | return r, nil | ||
| 121 | } | 121 | } | ||
| 122 | 122 | ||||
| 123 | - | func (r *Resource) ID() uint32 { return wlResourceGetID(r.c) } | 123 | + | func (r *Resource) Alive() bool { return !r.gone } |
| 124 | - | func (r *Resource) Version() int32 { return wlResourceGetVersion(r.c) } | 124 | + | func (r *Resource) ID() uint32 { |
| 125 | - | func (r *Resource) Client() Client { return Client{wlResourceGetClient(r.c)} } | 125 | + | if r.gone { |
| 126 | + | return 0 | |||
| 127 | + | } | |||
| 128 | + | return wlResourceGetID(r.c) | |||
| 129 | + | } | |||
| 130 | + | func (r *Resource) Version() int32 { | |||
| 131 | + | if r.gone { | |||
| 132 | + | return 0 | |||
| 133 | + | } | |||
| 134 | + | return wlResourceGetVersion(r.c) | |||
| 135 | + | } | |||
| 136 | + | func (r *Resource) Client() Client { | |||
| 137 | + | if r.gone { | |||
| 138 | + | return Client{} | |||
| 139 | + | } | |||
| 140 | + | return Client{wlResourceGetClient(r.c)} | |||
| 141 | + | } | |||
| 126 | func (r *Resource) Iface() *Interface { return r.iface } | 142 | func (r *Resource) Iface() *Interface { return r.iface } | ||
| 127 | 143 | ||||
| 128 | // Destroy frees the resource. It is a no-op once the resource is gone, so | 144 | // Destroy frees the resource. It is a no-op once the resource is gone, so | ||
| @@ -135,7 +151,11 @@ func (r *Resource) Destroy() { | |||||
| 135 | 151 | ||||
| 136 | // PostEvent sends an event. Arguments must match the event signature; build | 152 | // PostEvent sends an event. Arguments must match the event signature; build | ||
| 137 | // string and array arguments with a Pinner and unpin it after this returns. | 153 | // string and array arguments with a Pinner and unpin it after this returns. | ||
| 154 | + | // Calling it after destruction is safe and does nothing. | |||
| 138 | func (r *Resource) PostEvent(opcode uint32, args ...Arg) { | 155 | func (r *Resource) PostEvent(opcode uint32, args ...Arg) { | ||
| 156 | + | if r.gone { | |||
| 157 | + | return | |||
| 158 | + | } | |||
| 139 | var p unsafe.Pointer | 159 | var p unsafe.Pointer | ||
| 140 | if len(args) > 0 { | 160 | if len(args) > 0 { | ||
| 141 | p = unsafe.Pointer(&args[0]) | 161 | p = unsafe.Pointer(&args[0]) | ||
| @@ -150,7 +170,11 @@ func (r *Resource) PostEvent(opcode uint32, args ...Arg) { | |||||
| 150 | } | 170 | } | ||
| 151 | 171 | ||||
| 152 | // PostError sends a protocol error and disconnects the client. | 172 | // PostError sends a protocol error and disconnects the client. | ||
| 173 | + | // Calling it after destruction is safe and does nothing. | |||
| 153 | func (r *Resource) PostError(code uint32, msg string) { | 174 | func (r *Resource) PostError(code uint32, msg string) { | ||
| 175 | + | if r.gone { | |||
| 176 | + | return | |||
| 177 | + | } | |||
| 154 | // wl_resource_post_error takes a printf format; escape it and pass no | 178 | // wl_resource_post_error takes a printf format; escape it and pass no | ||
| 155 | // variadic arguments (purego clears AL, as SysV varargs require). | 179 | // variadic arguments (purego clears AL, as SysV varargs require). | ||
| 156 | b := append([]byte(strings.ReplaceAll(msg, "%", "%%")), 0) | 180 | b := append([]byte(strings.ReplaceAll(msg, "%", "%%")), 0) | ||