mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
fix(cron): prevent spin loop when job completes within firing second (#17821)
When a cron job fires at 13:00:00.014 and completes at 13:00:00.021, computeNextRunAtMs was flooring nowMs to 13:00:00.000 and asking croner for the next occurrence from that exact boundary. Croner could return 13:00:00.000 (same second) since it uses >= semantics, causing the job to be immediately re-triggered hundreds of times. Fix: Ask croner for the next occurrence starting from the NEXT second (e.g., 13:00:01.000). This ensures we always skip the current/elapsed second and correctly return the next day's occurrence. This also correctly handles the before-match case: if nowMs is 11:59:59.500, we ask from 12:00:00.000, and croner returns today's 12:00:00.000 match. Added regression tests for the spin loop scenario.
This commit is contained in:
committed by
Peter Steinberger
parent
531f735c8a
commit
de6cc05e7e
@@ -66,5 +66,23 @@ describe("cron schedule", () => {
|
||||
const next = computeNextRunAtMs(dailyNoon, noonMs - 500);
|
||||
expect(next).toBe(noonMs);
|
||||
});
|
||||
|
||||
it("advances to next day when job completes within same second it fired (#17821)", () => {
|
||||
// Regression test for #17821: cron jobs that fire and complete within
|
||||
// the same second (e.g., fire at 12:00:00.014, complete at 12:00:00.021)
|
||||
// were getting nextRunAtMs set to the same second, causing a spin loop.
|
||||
//
|
||||
// Simulating: job scheduled for 12:00:00, fires at .014, completes at .021
|
||||
const completedAtMs = noonMs + 21; // 12:00:00.021
|
||||
const next = computeNextRunAtMs(dailyNoon, completedAtMs);
|
||||
expect(next).toBe(noonMs + 86_400_000); // must be next day, NOT noonMs
|
||||
});
|
||||
|
||||
it("advances to next day when job completes just before second boundary (#17821)", () => {
|
||||
// Edge case: job completes at .999, still within the firing second
|
||||
const completedAtMs = noonMs + 999; // 12:00:00.999
|
||||
const next = computeNextRunAtMs(dailyNoon, completedAtMs);
|
||||
expect(next).toBe(noonMs + 86_400_000); // next day
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -49,19 +49,23 @@ export function computeNextRunAtMs(schedule: CronSchedule, nowMs: number): numbe
|
||||
timezone: resolveCronTimezone(schedule.tz),
|
||||
catch: false,
|
||||
});
|
||||
// Cron operates at second granularity, so floor nowMs to the start of the
|
||||
// current second. We ask croner for the next occurrence strictly *after*
|
||||
// nowSecondMs so that a job whose schedule matches the current second is
|
||||
// never re-scheduled into the same (already-elapsed) second.
|
||||
// Ask croner for the next occurrence starting from the NEXT second.
|
||||
// This prevents re-scheduling into the current second when a job fires
|
||||
// at 13:00:00.014 and completes at 13:00:00.021 — without this fix,
|
||||
// croner could return 13:00:00.000 (same second) causing a spin loop
|
||||
// where the job fires hundreds of times per second (see #17821).
|
||||
//
|
||||
// Previous code used `nowSecondMs - 1` which caused croner to return the
|
||||
// current second as a valid next-run, leading to rapid duplicate fires when
|
||||
// multiple jobs triggered simultaneously (see #14164).
|
||||
const nowSecondMs = Math.floor(nowMs / 1000) * 1000;
|
||||
const next = cron.nextRun(new Date(nowSecondMs));
|
||||
// By asking from the next second (e.g., 13:00:01.000), we ensure croner
|
||||
// returns the following day's occurrence (e.g., 13:00:00.000 tomorrow).
|
||||
//
|
||||
// This also correctly handles the "before match" case: if nowMs is
|
||||
// 11:59:59.500, we ask from 12:00:00.000, and croner returns 12:00:00.000
|
||||
// (today's match) since it uses >= semantics for the start time.
|
||||
const askFromNextSecondMs = Math.floor(nowMs / 1000) * 1000 + 1000;
|
||||
const next = cron.nextRun(new Date(askFromNextSecondMs));
|
||||
if (!next) {
|
||||
return undefined;
|
||||
}
|
||||
const nextMs = next.getTime();
|
||||
return Number.isFinite(nextMs) && nextMs > nowSecondMs ? nextMs : undefined;
|
||||
return Number.isFinite(nextMs) ? nextMs : undefined;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user