fix(core): keep zombies when a destroyed proxy unregisters late
Context.Unregister now removes only the proxy it was given, so a handler that was already running when its object was destroyed cannot delete the zombie and turn later in-flight events into ErrUnknownObject. dispatchFrame also re-checks the object table before calling a handler, so an object destroyed after lookup does not receive the event.
- commit
- 53f48a77c919d72e1971ed87c7a4b80d69b4eb04
- author
- brice · · 2026-09-29 15:52 UTC
- parent
- de4886b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +26 -2
M client.go +5 -0
| @@ -491,6 +491,11 @@ func (d *Display) dispatchFrame(f receivedFrame) error { | |||
| 491 | 491 | if _, isZombie := obj.(*zombie); isZombie { | |
| 492 | 492 | return nil | |
| 493 | 493 | } | |
| 494 | + | // Another goroutine may have destroyed the object after prepareFrame | |
| 495 | + | // looked it up; a destroyed object receives no further events. | |
| 496 | + | if current, _ := d.objects.Load(f.object); current != obj { | |
| 497 | + | return nil | |
| 498 | + | } | |
| 494 | 499 | if r, ok := obj.(eventReceiver); ok { | |
| 495 | 500 | r.Dispatch(ev) | |
| 496 | 501 | } | |
| @@ -491,6 +491,11 @@ func (d *Display) dispatchFrame(f receivedFrame) error { | |||||
| 491 | if _, isZombie := obj.(*zombie); isZombie { | 491 | if _, isZombie := obj.(*zombie); isZombie { | ||
| 492 | return nil | 492 | return nil | ||
| 493 | } | 493 | } | ||
| 494 | + | // Another goroutine may have destroyed the object after prepareFrame | |||
| 495 | + | // looked it up; a destroyed object receives no further events. | |||
| 496 | + | if current, _ := d.objects.Load(f.object); current != obj { | |||
| 497 | + | return nil | |||
| 498 | + | } | |||
| 494 | if r, ok := obj.(eventReceiver); ok { | 499 | if r, ok := obj.(eventReceiver); ok { | ||
| 495 | r.Dispatch(ev) | 500 | r.Dispatch(ev) | ||
| 496 | } | 501 | } | ||
M core.go +4 -2
| @@ -147,10 +147,12 @@ func (c *Context) Register(proxy Proxy) { | |||
| 147 | 147 | } | |
| 148 | 148 | ||
| 149 | 149 | // Unregister removes a proxy object | |
| 150 | + | // Only this proxy is removed: a zombie or a newer object that took the ID | |
| 151 | + | // is left in place. | |
| 150 | 152 | func (c *Context) Unregister(proxy Proxy) { | |
| 151 | 153 | if proxy != nil { | |
| 152 | - | c.proxies.Delete(proxy.ID()) | |
| 153 | - | c.display.objects.Delete(proxy.ID()) | |
| 154 | + | c.proxies.CompareAndDelete(proxy.ID(), proxy) | |
| 155 | + | c.display.objects.CompareAndDelete(proxy.ID(), proxy) | |
| 154 | 156 | } | |
| 155 | 157 | } | |
| 156 | 158 | ||
| @@ -147,10 +147,12 @@ func (c *Context) Register(proxy Proxy) { | |||||
| 147 | } | 147 | } | ||
| 148 | 148 | ||||
| 149 | // Unregister removes a proxy object | 149 | // Unregister removes a proxy object | ||
| 150 | + | // Only this proxy is removed: a zombie or a newer object that took the ID | |||
| 151 | + | // is left in place. | |||
| 150 | func (c *Context) Unregister(proxy Proxy) { | 152 | func (c *Context) Unregister(proxy Proxy) { | ||
| 151 | if proxy != nil { | 153 | if proxy != nil { | ||
| 152 | - | c.proxies.Delete(proxy.ID()) | 154 | + | c.proxies.CompareAndDelete(proxy.ID(), proxy) |
| 153 | - | c.display.objects.Delete(proxy.ID()) | 155 | + | c.display.objects.CompareAndDelete(proxy.ID(), proxy) |
| 154 | } | 156 | } | ||
| 155 | } | 157 | } | ||
| 156 | 158 | ||||
M hardening_test.go +17 -0
| @@ -160,3 +160,20 @@ func TestBindRecordsVersion(t *testing.T) { | |||
| 160 | 160 | t.Fatalf("version = %d, want 3", p.Version()) | |
| 161 | 161 | } | |
| 162 | 162 | } | |
| 163 | + | ||
| 164 | + | // A late Unregister of a destroyed proxy (for example from a handler that was | |
| 165 | + | // already running) must not remove its zombie. | |
| 166 | + | func TestUnregisterKeepsZombie(t *testing.T) { | |
| 167 | + | d := newDisplay(&chunkConn{}) | |
| 168 | + | p := &fdSigProxy{BaseProxy: BaseProxy{id: d.AllocateID(), context: d.context}} | |
| 169 | + | d.context.Register(p) | |
| 170 | + | if err := d.context.SendDestructor(p, 0); err != nil { | |
| 171 | + | t.Fatal(err) | |
| 172 | + | } | |
| 173 | + | d.context.Unregister(p) | |
| 174 | + | if obj, _ := d.objects.Load(p.ID()); obj == nil { | |
| 175 | + | t.Fatal("Unregister removed the zombie") | |
| 176 | + | } else if _, ok := obj.(*zombie); !ok { | |
| 177 | + | t.Fatalf("object = %T, want *zombie", obj) | |
| 178 | + | } | |
| 179 | + | } | |
| @@ -160,3 +160,20 @@ func TestBindRecordsVersion(t *testing.T) { | |||||
| 160 | t.Fatalf("version = %d, want 3", p.Version()) | 160 | t.Fatalf("version = %d, want 3", p.Version()) | ||
| 161 | } | 161 | } | ||
| 162 | } | 162 | } | ||
| 163 | + | ||||
| 164 | + | // A late Unregister of a destroyed proxy (for example from a handler that was | |||
| 165 | + | // already running) must not remove its zombie. | |||
| 166 | + | func TestUnregisterKeepsZombie(t *testing.T) { | |||
| 167 | + | d := newDisplay(&chunkConn{}) | |||
| 168 | + | p := &fdSigProxy{BaseProxy: BaseProxy{id: d.AllocateID(), context: d.context}} | |||
| 169 | + | d.context.Register(p) | |||
| 170 | + | if err := d.context.SendDestructor(p, 0); err != nil { | |||
| 171 | + | t.Fatal(err) | |||
| 172 | + | } | |||
| 173 | + | d.context.Unregister(p) | |||
| 174 | + | if obj, _ := d.objects.Load(p.ID()); obj == nil { | |||
| 175 | + | t.Fatal("Unregister removed the zombie") | |||
| 176 | + | } else if _, ok := obj.(*zombie); !ok { | |||
| 177 | + | t.Fatalf("object = %T, want *zombie", obj) | |||
| 178 | + | } | |||
| 179 | + | } | |||