Skip to content

Conversation

@ThomasK33
Copy link
Member

Fixes #172.

What changed

  • Replaced selection debouncing to use an explicit libuv timer (vim.uv/vim.loop) rather than vim.defer_fn().
  • Ensured debounce timers are always :stop()'d + :close()'d when cancelled or fired.
  • Guarded against stale timer callbacks (e.g. when a newer debounce supersedes an older one).
  • Ensured selection.disable() cancels any pending debounce/demotion timers.

Why

The previous code mixed vim.defer_fn() for creation with vim.loop.timer_stop() for cancellation and never closed the timer handle, which could lead to leaked handles and/or stale callbacks under rapid selection/cursor events.

Testing

  • nix develop .#ci -c luacheck lua/claudecode/selection.lua tests/selection_test.lua --no-unused-args --no-max-line-length
  • nix develop .#ci -c make test

📋 Implementation Plan

Plan: Fix issue #172 (timer API mismatch / unsafe debounce timer cleanup)

Context / Why

Issue #172 reports that lua/claudecode/selection.lua creates a debounce timer via vim.defer_fn() but cancels it via vim.loop.timer_stop(), and does not close() the timer handle. This can lead to inconsistent timer handling and (more importantly) leaked or double-closed timer handles under rapid events, which may contribute to crashes on newer Neovim versions.

Goal: Make selection debouncing use a single, consistent libuv timer API and ensure timers are always safely stopped + closed when cancelled or fired.

Evidence (verification)

  • GitHub issue Bug: Timer API mismatch in selection.lua - mixing vim.defer_fn with vim.loop.timer_stop #172 content (includes affected code, environment Neovim 0.11.5, suggested diff). (Fetched via tool.)
  • Current code confirms the reported pattern:
    • lua/claudecode/selection.lua:49 and :111 call vim.loop.timer_stop(M.state.debounce_timer).
    • lua/claudecode/selection.lua:114 assigns M.state.debounce_timer = vim.defer_fn(...).
  • Local Neovim probe (v0.12.0-dev in this workspace) confirms vim.defer_fn() returns a userdata timer handle with :stop() and :close() methods.

Plan

1) Implement a safe debounce timer using a libuv handle (no vim.defer_fn)

File: lua/claudecode/selection.lua

  • Replace the debounce implementation to use vim.loop.new_timer() (or local uv = vim.uv or vim.loop then uv.new_timer()), matching the rest of the module’s timer usage.
  • Key requirements:
    • Cancel path must stop() + close() the previous timer and set M.state.debounce_timer = nil before stopping/closing so any already-scheduled callback becomes a no-op.
    • Callback path must verify it is still the active timer (if M.state.debounce_timer ~= timer then return end) to avoid stale callbacks running after re-debounce.
    • Callback must stop() + close() the timer and clear state before calling M.update_selection().

Suggested structure (pseudocode):

  • M._cancel_debounce_timer() helper:
    • if M.state.debounce_timer then
      • local t = M.state.debounce_timer
      • M.state.debounce_timer = nil
      • t:stop(); t:close()
  • M.debounce_update():
    • call M._cancel_debounce_timer()
    • create timer = uv.new_timer()
    • M.state.debounce_timer = timer
    • timer:start(M.state.debounce_ms, 0, vim.schedule_wrap(function() ... end))

2) Fix disable() cleanup to stop + close the debounce timer

File: lua/claudecode/selection.lua

  • Replace the current vim.loop.timer_stop(M.state.debounce_timer) call with the same safe cancellation helper from step 1.
  • (Optional but recommended while touching cleanup): also cancel M.state.demotion_timer in disable() using the existing :stop(); :close(); nil pattern to prevent delayed callbacks firing after tracking is disabled.

3) Update/extend unit tests to cover the debounce contract

Files:

  • tests/selection_test.lua (standalone vim mock)
  • Potentially tests/mocks/vim.lua (shared mock) if needed

Add tests that validate:

  • Calling selection.debounce_update() twice rapidly cancels/cleans the previous timer (assert stop() and close() called once on the old handle).
  • A stale callback (from a cancelled timer) does not call update_selection().
  • selection.disable() cancels the active debounce timer without error.

To make this deterministic in unit tests, adjust the timer mock to:

  • store callback from timer:start(...) without auto-executing, and expose a timer:fire() helper for tests.

4) Regression check

  • Run make check (syntax + luacheck).
  • Run make test (busted).

Notes / Rationale

Why avoid “just change timer_stop to :stop()/:close()” while keeping vim.defer_fn?

vim.defer_fn() typically wraps a libuv timer and closes it internally when it fires. If we also close() the handle on cancel, there is a race where the timer fires, schedules its wrapper callback, then we cancel+close, and later the wrapper tries to close again. Implementing the debounce timer directly with uv.new_timer() gives us full control and enables a simple “stale timer” guard.


Generated with mux • Model: openai:gpt-5.2 • Thinking: xhigh

Change-Id: I4caa6d010f8f824aff1c38a7a73d08d47b400cce
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33
Copy link
Member Author

@codex review

@chatgpt-codex-connector
Copy link

To use Codex here, create a Codex account and connect to github.

@ThomasK33
Copy link
Member Author

@codex review

@chatgpt-codex-connector
Copy link

To use Codex here, create a Codex account and connect to github.

@ThomasK33
Copy link
Member Author

@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Swish!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Timer API mismatch in selection.lua - mixing vim.defer_fn with vim.loop.timer_stop

1 participant