fix(libinput): close keymaps of controls dropped at shutdown
A keymap received by the control relay but never applied, because Run stopped first, is now closed: the relay closes it when its hand-off is cancelled, and Run discards the controls still queued after the relay stops. The stats poll timeout is clamped at zero so an overdue deadline cannot turn into an endless wait. The idle sync waiter test bounds its waits and always cancels.
- commit
- 5f1215d544a7f539d5a94f05a64ecd6f465f7d80
- author
- brice · · 2026-10-14 04:11 UTC
- committer
- brice · · 2026-10-14 04:35 UTC
- parent
- 0c5c351
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +54 -19
M internal/adapters/libinput/libinput.go +41 -16
| @@ -237,12 +237,27 @@ type Options struct { | |||
| 237 | 237 | LogKeys bool | |
| 238 | 238 | } | |
| 239 | 239 | ||
| 240 | - | // relayControls hands the control messages of opts to Run as closures on | |
| 241 | - | // controls, waking its poll for each, until ctx ends. It only reads the | |
| 242 | - | // channels of opts; the closures run on Run's goroutine, the only one to | |
| 243 | - | // touch p, in, li and opts.Keymap. | |
| 244 | - | func relayControls(ctx context.Context, opts *Options, p *pointer, in *inputState, li uintptr, controls chan<- func(), wake func()) { | |
| 240 | + | // control is one control message of Options for Run to apply. keymap is the | |
| 241 | + | // replacement keymap it carries, if any: a control dropped at shutdown | |
| 242 | + | // closes it instead. | |
| 243 | + | type control struct { | |
| 244 | + | apply func() | |
| 245 | + | keymap *xkb.Keymap | |
| 246 | + | } | |
| 247 | + | ||
| 248 | + | func (c control) discard() { | |
| 249 | + | if c.keymap != nil { | |
| 250 | + | c.keymap.Close() | |
| 251 | + | } | |
| 252 | + | } | |
| 253 | + | ||
| 254 | + | // relayControls hands the control messages of opts to Run on controls, | |
| 255 | + | // waking its poll for each, until ctx ends. It only reads the channels of | |
| 256 | + | // opts; the controls run on Run's goroutine, the only one to touch p, in, li | |
| 257 | + | // and opts.Keymap. | |
| 258 | + | func relayControls(ctx context.Context, opts *Options, p *pointer, in *inputState, li uintptr, controls chan<- control, wake func()) { | |
| 245 | 259 | for { | |
| 260 | + | var c control | |
| 246 | 261 | var f func() | |
| 247 | 262 | select { | |
| 248 | 263 | case <-ctx.Done(): | |
| @@ -257,6 +272,7 @@ func relayControls(ctx context.Context, opts *Options, p *pointer, in *inputStat | |||
| 257 | 272 | opts.Log.Info().Bool("active", on).Msg("input") | |
| 258 | 273 | } | |
| 259 | 274 | case km := <-opts.Keymaps: | |
| 275 | + | c.keymap = km | |
| 260 | 276 | f = func() { | |
| 261 | 277 | if opts.Security != nil { | |
| 262 | 278 | km.QuarantineFrom(opts.Keymap) | |
| @@ -270,26 +286,28 @@ func relayControls(ctx context.Context, opts *Options, p *pointer, in *inputStat | |||
| 270 | 286 | p.setLayout(l) | |
| 271 | 287 | p.moved(opts.MoveCursor, false) | |
| 272 | 288 | } | |
| 273 | - | case c := <-opts.Constraints: | |
| 289 | + | case pc := <-opts.Constraints: | |
| 274 | 290 | f = func() { | |
| 275 | - | p.constrain(c) | |
| 291 | + | p.constrain(pc) | |
| 276 | 292 | p.moved(opts.MoveCursor, false) | |
| 277 | 293 | } | |
| 278 | - | case c := <-opts.DeviceConfigs: | |
| 294 | + | case dc := <-opts.DeviceConfigs: | |
| 279 | 295 | f = func() { | |
| 280 | - | in.cfg = c | |
| 296 | + | in.cfg = dc | |
| 281 | 297 | for dev := range in.devices { | |
| 282 | 298 | in.configure(dev, opts.Log) | |
| 283 | 299 | } | |
| 284 | - | t, m := c.Touchpad, c.Mouse | |
| 300 | + | t, m := dc.Touchpad, dc.Mouse | |
| 285 | 301 | opts.Log.Info().Bool("natural_scroll", t.NaturalScroll).Bool("tap", t.Tap).Float64("accel_speed", t.AccelSpeed).Str("accel_profile", t.AccelProfile).Bool("left_handed", t.LeftHanded).Float64("scroll_factor", t.ScrollFactor). | |
| 286 | 302 | Bool("mouse_natural_scroll", m.NaturalScroll).Float64("mouse_accel_speed", m.AccelSpeed).Str("mouse_accel_profile", m.AccelProfile).Bool("mouse_left_handed", m.LeftHanded).Msg("input devices reloaded") | |
| 287 | 303 | } | |
| 288 | 304 | } | |
| 305 | + | c.apply = f | |
| 289 | 306 | select { | |
| 290 | - | case controls <- f: | |
| 307 | + | case controls <- c: | |
| 291 | 308 | wake() | |
| 292 | 309 | case <-ctx.Done(): | |
| 310 | + | c.discard() | |
| 293 | 311 | return | |
| 294 | 312 | } | |
| 295 | 313 | } | |
| @@ -342,9 +360,16 @@ func Run(ctx context.Context, opts Options, input chan<- ports.InputEvent) error | |||
| 342 | 360 | fwdDone.Go(func() { fwd.run(fwdCtx, input) }) | |
| 343 | 361 | // Control messages wake the poll instead of a polling timeout; they are | |
| 344 | 362 | // applied on this goroutine, which owns libinput. | |
| 345 | - | controls := make(chan func(), 8) | |
| 363 | + | controls := make(chan control, 8) | |
| 346 | 364 | fwdDone.Go(func() { relayControls(fwdCtx, &opts, p, in, li, controls, waiter.Wake) }) | |
| 347 | - | defer func() { stopFwd(); fwdDone.Wait() }() | |
| 365 | + | defer func() { | |
| 366 | + | stopFwd() | |
| 367 | + | fwdDone.Wait() | |
| 368 | + | // Controls never applied: free what they carry. | |
| 369 | + | for len(controls) > 0 { | |
| 370 | + | (<-controls).discard() | |
| 371 | + | } | |
| 372 | + | }() | |
| 348 | 373 | fds := []unix.PollFd{{Fd: fd, Events: unix.POLLIN}} | |
| 349 | 374 | nextStats := time.Now().Add(statsEvery) | |
| 350 | 375 | for ctx.Err() == nil { | |
| @@ -357,13 +382,13 @@ func Run(ctx context.Context, opts Options, input chan<- ports.InputEvent) error | |||
| 357 | 382 | apply: | |
| 358 | 383 | for { | |
| 359 | 384 | select { | |
| 360 | - | case f := <-controls: | |
| 361 | - | f() | |
| 385 | + | case c := <-controls: | |
| 386 | + | c.apply() | |
| 362 | 387 | default: | |
| 363 | 388 | break apply | |
| 364 | 389 | } | |
| 365 | 390 | } | |
| 366 | - | n, err := waiter.Poll(fds, int(time.Until(nextStats).Milliseconds())+1) | |
| 391 | + | n, err := waiter.Poll(fds, max(0, int(time.Until(nextStats).Milliseconds())+1)) | |
| 367 | 392 | if err != nil && !errors.Is(err, unix.EINTR) { | |
| 368 | 393 | return fmt.Errorf("libinput poll: %w", err) | |
| 369 | 394 | } | |
| @@ -237,12 +237,27 @@ type Options struct { | |||||
| 237 | LogKeys bool | 237 | LogKeys bool | ||
| 238 | } | 238 | } | ||
| 239 | 239 | ||||
| 240 | - | // relayControls hands the control messages of opts to Run as closures on | 240 | + | // control is one control message of Options for Run to apply. keymap is the |
| 241 | - | // controls, waking its poll for each, until ctx ends. It only reads the | 241 | + | // replacement keymap it carries, if any: a control dropped at shutdown |
| 242 | - | // channels of opts; the closures run on Run's goroutine, the only one to | 242 | + | // closes it instead. |
| 243 | - | // touch p, in, li and opts.Keymap. | 243 | + | type control struct { |
| 244 | - | func relayControls(ctx context.Context, opts *Options, p *pointer, in *inputState, li uintptr, controls chan<- func(), wake func()) { | 244 | + | apply func() |
| 245 | + | keymap *xkb.Keymap | |||
| 246 | + | } | |||
| 247 | + | ||||
| 248 | + | func (c control) discard() { | |||
| 249 | + | if c.keymap != nil { | |||
| 250 | + | c.keymap.Close() | |||
| 251 | + | } | |||
| 252 | + | } | |||
| 253 | + | ||||
| 254 | + | // relayControls hands the control messages of opts to Run on controls, | |||
| 255 | + | // waking its poll for each, until ctx ends. It only reads the channels of | |||
| 256 | + | // opts; the controls run on Run's goroutine, the only one to touch p, in, li | |||
| 257 | + | // and opts.Keymap. | |||
| 258 | + | func relayControls(ctx context.Context, opts *Options, p *pointer, in *inputState, li uintptr, controls chan<- control, wake func()) { | |||
| 245 | for { | 259 | for { | ||
| 260 | + | var c control | |||
| 246 | var f func() | 261 | var f func() | ||
| 247 | select { | 262 | select { | ||
| 248 | case <-ctx.Done(): | 263 | case <-ctx.Done(): | ||
| @@ -257,6 +272,7 @@ func relayControls(ctx context.Context, opts *Options, p *pointer, in *inputStat | |||||
| 257 | opts.Log.Info().Bool("active", on).Msg("input") | 272 | opts.Log.Info().Bool("active", on).Msg("input") | ||
| 258 | } | 273 | } | ||
| 259 | case km := <-opts.Keymaps: | 274 | case km := <-opts.Keymaps: | ||
| 275 | + | c.keymap = km | |||
| 260 | f = func() { | 276 | f = func() { | ||
| 261 | if opts.Security != nil { | 277 | if opts.Security != nil { | ||
| 262 | km.QuarantineFrom(opts.Keymap) | 278 | km.QuarantineFrom(opts.Keymap) | ||
| @@ -270,26 +286,28 @@ func relayControls(ctx context.Context, opts *Options, p *pointer, in *inputStat | |||||
| 270 | p.setLayout(l) | 286 | p.setLayout(l) | ||
| 271 | p.moved(opts.MoveCursor, false) | 287 | p.moved(opts.MoveCursor, false) | ||
| 272 | } | 288 | } | ||
| 273 | - | case c := <-opts.Constraints: | 289 | + | case pc := <-opts.Constraints: |
| 274 | f = func() { | 290 | f = func() { | ||
| 275 | - | p.constrain(c) | 291 | + | p.constrain(pc) |
| 276 | p.moved(opts.MoveCursor, false) | 292 | p.moved(opts.MoveCursor, false) | ||
| 277 | } | 293 | } | ||
| 278 | - | case c := <-opts.DeviceConfigs: | 294 | + | case dc := <-opts.DeviceConfigs: |
| 279 | f = func() { | 295 | f = func() { | ||
| 280 | - | in.cfg = c | 296 | + | in.cfg = dc |
| 281 | for dev := range in.devices { | 297 | for dev := range in.devices { | ||
| 282 | in.configure(dev, opts.Log) | 298 | in.configure(dev, opts.Log) | ||
| 283 | } | 299 | } | ||
| 284 | - | t, m := c.Touchpad, c.Mouse | 300 | + | t, m := dc.Touchpad, dc.Mouse |
| 285 | opts.Log.Info().Bool("natural_scroll", t.NaturalScroll).Bool("tap", t.Tap).Float64("accel_speed", t.AccelSpeed).Str("accel_profile", t.AccelProfile).Bool("left_handed", t.LeftHanded).Float64("scroll_factor", t.ScrollFactor). | 301 | opts.Log.Info().Bool("natural_scroll", t.NaturalScroll).Bool("tap", t.Tap).Float64("accel_speed", t.AccelSpeed).Str("accel_profile", t.AccelProfile).Bool("left_handed", t.LeftHanded).Float64("scroll_factor", t.ScrollFactor). | ||
| 286 | Bool("mouse_natural_scroll", m.NaturalScroll).Float64("mouse_accel_speed", m.AccelSpeed).Str("mouse_accel_profile", m.AccelProfile).Bool("mouse_left_handed", m.LeftHanded).Msg("input devices reloaded") | 302 | Bool("mouse_natural_scroll", m.NaturalScroll).Float64("mouse_accel_speed", m.AccelSpeed).Str("mouse_accel_profile", m.AccelProfile).Bool("mouse_left_handed", m.LeftHanded).Msg("input devices reloaded") | ||
| 287 | } | 303 | } | ||
| 288 | } | 304 | } | ||
| 305 | + | c.apply = f | |||
| 289 | select { | 306 | select { | ||
| 290 | - | case controls <- f: | 307 | + | case controls <- c: |
| 291 | wake() | 308 | wake() | ||
| 292 | case <-ctx.Done(): | 309 | case <-ctx.Done(): | ||
| 310 | + | c.discard() | |||
| 293 | return | 311 | return | ||
| 294 | } | 312 | } | ||
| 295 | } | 313 | } | ||
| @@ -342,9 +360,16 @@ func Run(ctx context.Context, opts Options, input chan<- ports.InputEvent) error | |||||
| 342 | fwdDone.Go(func() { fwd.run(fwdCtx, input) }) | 360 | fwdDone.Go(func() { fwd.run(fwdCtx, input) }) | ||
| 343 | // Control messages wake the poll instead of a polling timeout; they are | 361 | // Control messages wake the poll instead of a polling timeout; they are | ||
| 344 | // applied on this goroutine, which owns libinput. | 362 | // applied on this goroutine, which owns libinput. | ||
| 345 | - | controls := make(chan func(), 8) | 363 | + | controls := make(chan control, 8) |
| 346 | fwdDone.Go(func() { relayControls(fwdCtx, &opts, p, in, li, controls, waiter.Wake) }) | 364 | fwdDone.Go(func() { relayControls(fwdCtx, &opts, p, in, li, controls, waiter.Wake) }) | ||
| 347 | - | defer func() { stopFwd(); fwdDone.Wait() }() | 365 | + | defer func() { |
| 366 | + | stopFwd() | |||
| 367 | + | fwdDone.Wait() | |||
| 368 | + | // Controls never applied: free what they carry. | |||
| 369 | + | for len(controls) > 0 { | |||
| 370 | + | (<-controls).discard() | |||
| 371 | + | } | |||
| 372 | + | }() | |||
| 348 | fds := []unix.PollFd{{Fd: fd, Events: unix.POLLIN}} | 373 | fds := []unix.PollFd{{Fd: fd, Events: unix.POLLIN}} | ||
| 349 | nextStats := time.Now().Add(statsEvery) | 374 | nextStats := time.Now().Add(statsEvery) | ||
| 350 | for ctx.Err() == nil { | 375 | for ctx.Err() == nil { | ||
| @@ -357,13 +382,13 @@ func Run(ctx context.Context, opts Options, input chan<- ports.InputEvent) error | |||||
| 357 | apply: | 382 | apply: | ||
| 358 | for { | 383 | for { | ||
| 359 | select { | 384 | select { | ||
| 360 | - | case f := <-controls: | 385 | + | case c := <-controls: |
| 361 | - | f() | 386 | + | c.apply() |
| 362 | default: | 387 | default: | ||
| 363 | break apply | 388 | break apply | ||
| 364 | } | 389 | } | ||
| 365 | } | 390 | } | ||
| 366 | - | n, err := waiter.Poll(fds, int(time.Until(nextStats).Milliseconds())+1) | 391 | + | n, err := waiter.Poll(fds, max(0, int(time.Until(nextStats).Milliseconds())+1)) |
| 367 | if err != nil && !errors.Is(err, unix.EINTR) { | 392 | if err != nil && !errors.Is(err, unix.EINTR) { | ||
| 368 | return fmt.Errorf("libinput poll: %w", err) | 393 | return fmt.Errorf("libinput poll: %w", err) | ||
| 369 | } | 394 | } | ||
M internal/adapters/wayland/acquire_failure_test.go +13 -3
| @@ -136,13 +136,23 @@ func TestSyncWaiterIdleNeverTicks(t *testing.T) { | |||
| 136 | 136 | return real.poll(fds, timeout) | |
| 137 | 137 | }) | |
| 138 | 138 | ctx, cancel := context.WithCancel(context.Background()) | |
| 139 | + | defer cancel() | |
| 139 | 140 | done := make(chan struct{}) | |
| 140 | 141 | go func() { sw.run(ctx); close(done) }() | |
| 141 | - | if timeout := <-polls; timeout != -1 { | |
| 142 | - | t.Fatalf("idle poll timeout %d, want -1", timeout) | |
| 142 | + | select { | |
| 143 | + | case timeout := <-polls: | |
| 144 | + | if timeout != -1 { | |
| 145 | + | t.Fatalf("idle poll timeout %d, want -1", timeout) | |
| 146 | + | } | |
| 147 | + | case <-time.After(5 * time.Second): | |
| 148 | + | t.Fatal("the waiter never polled") | |
| 143 | 149 | } | |
| 144 | 150 | cancel() | |
| 145 | - | <-done | |
| 151 | + | select { | |
| 152 | + | case <-done: | |
| 153 | + | case <-time.After(5 * time.Second): | |
| 154 | + | t.Fatal("cancel did not stop the waiter") | |
| 155 | + | } | |
| 146 | 156 | if n := len(polls); n != 0 { | |
| 147 | 157 | t.Fatalf("%d extra polls while idle", n) | |
| 148 | 158 | } | |
| @@ -136,13 +136,23 @@ func TestSyncWaiterIdleNeverTicks(t *testing.T) { | |||||
| 136 | return real.poll(fds, timeout) | 136 | return real.poll(fds, timeout) | ||
| 137 | }) | 137 | }) | ||
| 138 | ctx, cancel := context.WithCancel(context.Background()) | 138 | ctx, cancel := context.WithCancel(context.Background()) | ||
| 139 | + | defer cancel() | |||
| 139 | done := make(chan struct{}) | 140 | done := make(chan struct{}) | ||
| 140 | go func() { sw.run(ctx); close(done) }() | 141 | go func() { sw.run(ctx); close(done) }() | ||
| 141 | - | if timeout := <-polls; timeout != -1 { | 142 | + | select { |
| 142 | - | t.Fatalf("idle poll timeout %d, want -1", timeout) | 143 | + | case timeout := <-polls: |
| 144 | + | if timeout != -1 { | |||
| 145 | + | t.Fatalf("idle poll timeout %d, want -1", timeout) | |||
| 146 | + | } | |||
| 147 | + | case <-time.After(5 * time.Second): | |||
| 148 | + | t.Fatal("the waiter never polled") | |||
| 143 | } | 149 | } | ||
| 144 | cancel() | 150 | cancel() | ||
| 145 | - | <-done | 151 | + | select { |
| 152 | + | case <-done: | |||
| 153 | + | case <-time.After(5 * time.Second): | |||
| 154 | + | t.Fatal("cancel did not stop the waiter") | |||
| 155 | + | } | |||
| 146 | if n := len(polls); n != 0 { | 156 | if n := len(polls); n != 0 { | ||
| 147 | t.Fatalf("%d extra polls while idle", n) | 157 | t.Fatalf("%d extra polls while idle", n) | ||
| 148 | } | 158 | } | ||