Prevent double-free of the module timer when the callback stops it

[quanyeyang]

Summary

• Fix a double free in moduleTimerHandler() when a module timer callback calls ValkeyModule_StopTimer() on the currently firing timer (VM_StopTimer frees the object, then the dispatcher freed it again).
• After the callback returns, only remove/free the timer if the original timer ID still maps to the same live object in Timers.
• Add a module API regression test that stops the firing timer from inside its callback.

Fixes #4200

Test plan

• ./runtest --single unit/moduleapi/timer

[coderabbitai]

📝 Walkthrough

Walkthrough

Module timer dispatch now safely handles callbacks that stop their own timers. The timer test module adds a self-stopping command and unit coverage verifying callback execution, timer removal, and continued server responsiveness.

Changes

Self-stopping timer handling

Layer / File(s)	Summary
Guard timer cleanup after callbacks src/module.c	Timer dispatch preserves the timer ID before invoking the callback and only removes and frees the matching timer entry if it remains in the radix tree.
Add self-stopping timer coverage tests/modules/timer.c, tests/unit/moduleapi/timer.tcl	The test module adds test.selfstoptimer, whose callback stops its own timer and updates a key; the unit test verifies the timer is removed and the server remains responsive.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address #4200 by preserving the timer ID, avoiding կրկ cleanup after StopTimer, and keeping callback timer operations working.
Out of Scope Changes check	✅ Passed	The added command and regression test are directly related to the timer self-stop fix and do not appear out of scope.
Title check	✅ Passed	The title clearly matches the main change: preventing a double-free when a timer callback stops its own timer.
Description check	✅ Passed	The description directly describes the bug fix, the handler change, and the regression test added in this PR.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.81%. Comparing base (6b006c9) to head (367cf63).

Files with missing lines	Patch %	Lines
src/module.c	0.00%	6 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #4211      +/-   ##
============================================
- Coverage     76.81%   76.81%   -0.01%     
============================================
  Files           162      162              
  Lines         81455    81459       +4     
============================================
  Hits          62570    62570              
- Misses        18885    18889       +4     

Files with missing lines	Coverage Δ
src/module.c	25.43% <0.00%> (-0.02%)	⬇️

... and 18 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[valkey-review-bot]

I found one gap in the new regression coverage for the self-stop path.

[enjoy-binbin]

Thanks, look good.