Skip to content

fix(cron): fix scheduling bugs and add destroy method#97

Open
hakanesnn wants to merge 6 commits intoCommunityOx:mainfrom
hakanesnn:fix/cron-scheduling
Open

fix(cron): fix scheduling bugs and add destroy method#97
hakanesnn wants to merge 6 commits intoCommunityOx:mainfrom
hakanesnn:fix/cron-scheduling

Conversation

@hakanesnn
Copy link
Copy Markdown

Fixes

  • getMaxDaysInMonth: Fixed off-by-one error by using day=0. Previously returned 30 for January, 27 for February, etc.
  • getNextTime logic: Rewritten to scan forward for the next execution time. This fixes:
  1. Hour ranges (e.g., 0 17-23 * * *) firing only once and stopping (Issue: [Bug] Cron - Day ahead or more cron doesn't run #77).
  2. Future-dated crons (e.g., 0 0 01 01 *) failing to schedule if the target date is not today (Issue: [Bug] Cron not working properly #95).

Improvements

  • task:destroy(): New method to stop tasks and remove them from the registry, ensuring they can be garbage collected.
  • Task ID management: Switched to an incrementing counter instead of #tasks+1 to prevent ID collisions when tasks are destroyed.
  • manualStop flag: Prevents the midnight reschedule job from restarting tasks that were intentionally stopped by the user.
  • Code Cleanup: Removed ~70 lines of redundant code in getAbsoluteNextTime by delegating logic to the updated getNextTime.
Test Code
-- Test 1: Hour range - uses current hour so minute overflow always triggers the bug
-- OLD: returns current hour's :00 (already past), task stops
-- NEW: returns next hour's :00 within the range
local currentHour = tonumber(os.date('%H'))
local rangeEnd = math.min(currentHour + 2, 23)
local hourExpr = ('0 %d-%d * * *'):format(currentHour, rangeEnd)
local t1 = lib.cron.new(hourExpr, function() end, { debug = true })
local next1 = t1:getNextTime()
print(('[Test 1 - Hour Range] expression: %s | isActive: %s | nextTime: %s'):format(
    hourExpr,
    tostring(t1.isActive),
    next1 and os.date('%c', next1) or 'nil'
))
t1:stop()

-- Test 2: Future day - schedules for tomorrow
-- OLD: returns nil because day != today
-- NEW: returns tomorrow's date
local tomorrow = os.date('*t', os.time() + 86400)
local expr = ('0 0 %d %d *'):format(tomorrow.day, tomorrow.month)
local t2 = lib.cron.new(expr, function() end, { debug = true })
local next2 = t2:getNextTime()
print(('[Test 2 - Future Day] expression: %s | isActive: %s | nextTime: %s'):format(
    expr,
    tostring(t2.isActive),
    next2 and os.date('%c', next2) or 'nil'
))
t2:stop()

-- Test 3: getMaxDaysInMonth - verifies day=0 fix
-- day=-1 returns second-to-last day, day=0 returns correct last day
local jan_old = os.date('*t', os.time({ year = 2026, month = 2, day = -1 })).day
local feb_old = os.date('*t', os.time({ year = 2026, month = 3, day = -1 })).day
local apr_old = os.date('*t', os.time({ year = 2026, month = 5, day = -1 })).day
local jan_new = os.date('*t', os.time({ year = 2026, month = 2, day = 0 })).day
local feb_new = os.date('*t', os.time({ year = 2026, month = 3, day = 0 })).day
local apr_new = os.date('*t', os.time({ year = 2026, month = 5, day = 0 })).day
print(('[Test 3 - Max Days] day=-1: Jan=%d Feb=%d Apr=%d | day=0: Jan=%d Feb=%d Apr=%d'):format(
    jan_old, feb_old, apr_old, jan_new, feb_new, apr_new
))

-- Test 4: Last day of month via L expression
-- OLD: nil (day != today + off-by-one)
-- NEW: correct last day of current month
local tL = lib.cron.new('0 0 L * *', function() end, { debug = true })
local nextL = tL:getNextTime()
print(('[Test 4 - Last Day (L)] nextTime: %s'):format(
    nextL and os.date('%c', nextL) or 'nil'
))
tL:stop()

-- Test 5: Task ID uniqueness after destroy
-- OLD: destroy() does not exist
-- NEW: IDs never collide after destroy
if t1.destroy then
    local t3 = lib.cron.new('0 0 * * *', function() end)
    local t4 = lib.cron.new('0 0 * * *', function() end)
    local id3, id4 = t3.id, t4.id
    t3:destroy()
    local t5 = lib.cron.new('0 0 * * *', function() end)
    print(('[Test 5 - Task ID] id3=%d id4=%d id5=%d | no_collision=%s'):format(
        id3, id4, t5.id,
        tostring(t5.id ~= id3 and t5.id ~= id4)
    ))
    t5:destroy()
    t4:destroy()
else
    print('[Test 5 - Task ID] skipped (destroy not available)')
end

-- Test 6: Manual stop flag
-- OLD: manualStop does not exist
-- NEW: manualStop=true when user calls stop()
local t6 = lib.cron.new('0 0 * * *', function() end)
t6:stop()
if t6.manualStop ~= nil then
    print(('[Test 6 - Manual Stop] manualStop: %s'):format(tostring(t6.manualStop)))
    if t6.destroy then t6:destroy() end
else
    print('[Test 6 - Manual Stop] skipped (manualStop not available)')
end

Old code:

[Test 1 - Hour Range]  expression: 0 13-15 * * * | nextTime: Mon Apr  6 13:00:00 2026  (past time, task dies)
[Test 2 - Future Day]  expression: 0 0 7 4 *    | nextTime: nil
[Test 3 - Max Days]    day=-1: Jan=30 Feb=27 Apr=29 | day=0: Jan=31 Feb=28 Apr=30
[Test 4 - Last Day (L)]                             | nextTime: nil
[Test 5 - Task ID]     skipped (destroy not available)
[Test 6 - Manual Stop] skipped (manualStop not available)

New code:

[Test 1 - Hour Range]  expression: 0 14-16 * * * | nextTime: Mon Apr  6 15:00:00 2026
[Test 2 - Future Day]  expression: 0 0 7 4 *    | nextTime: Tue Apr  7 00:00:00 2026
[Test 3 - Max Days]    day=-1: Jan=30 Feb=27 Apr=29 | day=0: Jan=31 Feb=28 Apr=30
[Test 4 - Last Day (L)]                             | nextTime: Thu Apr 30 00:00:00 2026
[Test 5 - Task ID]     id3=5 id4=6 id5=7 | no_collision=true
[Test 6 - Manual Stop] manualStop: true

…scheduling

getNextTime only checked if the current day matched, causing cron jobs
with future dates or hour ranges to silently stop. Rewrote it to scan
up to 366 days ahead, checking each candidate day/hour/minute against
the cron fields via a new matchesField helper function.

Fixes hour range expressions (e.g. 0 17-23 * * sun) only firing once,
and future day expressions (e.g. 0 0 01 01 *) never firing.
getAbsoluteNextTime now delegates to the rewritten getNextTime instead
of duplicating time calculation logic. Removes the now-unused
getTimeUnit function and maxUnits table.
The daily midnight job previously restarted all inactive tasks,
including ones intentionally stopped by the user. Added a manualStop
flag to distinguish user-initiated stops from internal ones, so only
internally stopped tasks get rescheduled.
task:destroy() stops the task and removes it from the internal tasks
table, allowing garbage collection. Also switched the midnight
reschedule loop to use pairs() since destroyed tasks leave nil gaps.
day=-1 returns the second-to-last day of the month, not the last.
Changed to day=0 which correctly returns the last day.
#tasks+1 is unreliable for sparse tables after destroy() leaves nil
gaps. Replaced with a monotonically incrementing counter to guarantee
unique IDs.
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.

2 participants