From 6502a6f78074e14862113ba8703bd5d98105f13e Mon Sep 17 00:00:00 2001 From: Rahul Rao Date: Sat, 11 Jul 2026 22:32:49 +0530 Subject: [PATCH 1/2] Align Windows _jobs_lock timeout with POSIX polling pattern The POSIX (fcntl) path polls LOCK_NB for 30 seconds with explicit timeout logging before degrading to in-process-only locking (fix for issue #60703). The Windows (msvcrt) path used a blocking LK_LOCK with ~10s internal retry and a generic warning on failure. Replace LK_LOCK with LK_NBLCK in a polling loop matching the POSIX pattern (same 30-second _JOBS_LOCK_TIMEOUT_SECONDS, same degradation behavior, same ERROR-level log message). This ensures consistent timeout handling across platforms and gives the Windows path the same bounded contention resilience that POSIX has. Signed-off-by: Rahul Rao --- cron/jobs.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/cron/jobs.py b/cron/jobs.py index 356385cdce78b..8e34e6a26e353 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -328,7 +328,29 @@ def _jobs_lock(): break time.sleep(0.1) elif msvcrt is not None: - getattr(msvcrt, "locking")(lock_fd.fileno(), getattr(msvcrt, "LK_LOCK"), 1) + _deadline = time.monotonic() + _JOBS_LOCK_TIMEOUT_SECONDS + while True: + try: + lock_fd.seek(0) + getattr(msvcrt, "locking")(lock_fd.fileno(), getattr(msvcrt, "LK_NBLCK"), 1) + break + except OSError: + if time.monotonic() >= _deadline: + logger.error( + "Timed out after %.0fs waiting for the cron " + "jobs lock (%s) — another process is holding " + "it. Proceeding with in-process locking only " + "so the scheduler stays alive (#60703).", + _JOBS_LOCK_TIMEOUT_SECONDS, + _jobs_lock_file(), + ) + try: + lock_fd.close() + except OSError: + pass + lock_fd = None + break + time.sleep(0.1) except (OSError, IOError) as e: # Never let a locking failure take down cron writes — fall back to # in-process-only protection (still held via _jobs_file_lock). From b828a04ee0c8fd678affcfe5efe6d0d384e68802 Mon Sep 17 00:00:00 2001 From: Rahul Rao Date: Sat, 11 Jul 2026 23:58:40 +0530 Subject: [PATCH 2/2] Add mocked-msvcrt regression tests for Windows _jobs_lock path The bounded-lock tests in test_ticker_stall_60703.py are module-skipped when fcntl is unavailable, so they never cover the Windows msvcrt path. Add a new test file with a fake msvcrt module that exercises LK_NBLCK polling on any platform: - test_lock_times_out_and_degrades: always-fail mock verifies 30-second bounded timeout, ERROR logging, and degraded critical-section entry. - test_uncontended_lock_is_fast_and_silent: immediate LK_NBLCK success. - test_lock_recovers_after_transient_contention: 1 failure then success must not trigger the timeout path. Signed-off-by: Rahul Rao --- tests/cron/test_windows_jobs_lock.py | 87 ++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 tests/cron/test_windows_jobs_lock.py diff --git a/tests/cron/test_windows_jobs_lock.py b/tests/cron/test_windows_jobs_lock.py new file mode 100644 index 0000000000000..e9a86e1f57ee7 --- /dev/null +++ b/tests/cron/test_windows_jobs_lock.py @@ -0,0 +1,87 @@ +"""Regression tests for the Windows (msvcrt) _jobs_lock path. + +The POSIX bounded-lock fix (#60703, #60855) added a polling loop with +30-second timeout and graceful degradation for fcntl/flock, but the +msvcrt/Windows path was not updated — it used a blocking LK_LOCK call +with ~10s internal retry and a generic WARNING on failure. + +These tests verify that the msvcrt path now uses LK_NBLCK with the same +30-second bounded polling, ERROR-level timeout logging, and degraded +critical-section entry as the POSIX path. They run on any platform by +faking an msvcrt module (since real msvcrt is only available on Windows). +""" + +import time + +import cron.jobs as jobs_mod + + +def _fake_msvcrt(*, fail_count: int = 0): + """Build a fake ``msvcrt`` module for ``_jobs_lock``. + + *fail_count*: how many ``locking()`` calls raise ``OSError`` before + succeeding. ``-1`` means always fail. + """ + attempts = [0] # mutable closure + + class FakeMsvcrt: + LK_NBLCK = 0x0004 + LK_UNLCK = 0x0000 + + @staticmethod + def locking(fd, mode, nbytes): + if fail_count == -1: + raise OSError(33, "lock held by another process") + attempts[0] += 1 + if attempts[0] <= fail_count: + raise OSError(33, "lock held by another process") + + return FakeMsvcrt + + +class TestWindowsBoundedJobsLock: + def test_lock_times_out_and_degrades(self, monkeypatch, caplog): + """LK_NBLCK contention must not block _jobs_lock forever on Windows.""" + monkeypatch.setattr(jobs_mod, "fcntl", None) + monkeypatch.setattr(jobs_mod, "msvcrt", _fake_msvcrt(fail_count=-1)) + monkeypatch.setattr(jobs_mod, "_JOBS_LOCK_TIMEOUT_SECONDS", 1.0) + jobs_mod.ensure_dirs() + _ = jobs_mod._jobs_lock_file().touch() + + start = time.monotonic() + entered = False + with caplog.at_level("ERROR", logger="cron.jobs"): + with jobs_mod._jobs_lock(): + entered = True + elapsed = time.monotonic() - start + + assert entered, "critical section must still run in degraded mode" + assert elapsed < 10, f"lock wait was not bounded (took {elapsed:.1f}s)" + assert any("Timed out" in r.message for r in caplog.records), ( + "degraded-mode fallback must be logged at ERROR" + ) + + def test_uncontended_lock_is_fast_and_silent(self, monkeypatch, caplog): + """Uncontested LK_NBLCK acquisition should succeed immediately.""" + monkeypatch.setattr(jobs_mod, "fcntl", None) + monkeypatch.setattr(jobs_mod, "msvcrt", _fake_msvcrt(fail_count=0)) + jobs_mod.ensure_dirs() + + start = time.monotonic() + with caplog.at_level("ERROR", logger="cron.jobs"): + with jobs_mod._jobs_lock(): + pass + assert time.monotonic() - start < 5 + assert not [r for r in caplog.records if "Timed out" in r.message] + + def test_lock_recovers_after_transient_contention(self, monkeypatch, caplog): + """A brief LK_NBLCK conflict (1 failure, then success) must not degrade.""" + monkeypatch.setattr(jobs_mod, "fcntl", None) + monkeypatch.setattr(jobs_mod, "msvcrt", _fake_msvcrt(fail_count=1)) + monkeypatch.setattr(jobs_mod, "_JOBS_LOCK_TIMEOUT_SECONDS", 3.0) + jobs_mod.ensure_dirs() + + with caplog.at_level("ERROR", logger="cron.jobs"): + with jobs_mod._jobs_lock(): + pass + assert not [r for r in caplog.records if "Timed out" in r.message]