Skip to content

test: reproduce #198 — setup_schedule runs before lock acquisition - #327

Open
sibson wants to merge 1 commit into
mainfrom
claude/triage-issue-198
Open

test: reproduce #198 — setup_schedule runs before lock acquisition#327
sibson wants to merge 1 commit into
mainfrom
claude/triage-issue-198

Conversation

@sibson

@sibson sibson commented Jul 27, 2026

Copy link
Copy Markdown
Owner

What this is

A failing regression test for #198, not a fix. Confirms the report: setup_schedule() writes static entries and the :statics set unconditionally, and runs from RedBeatScheduler.__init__ (the base Scheduler constructor is non-lazy) — which happens before beat_init fires and before self.lock is ever set. A second instance started with a different beat_schedule overwrites the statics set the lock-holding instance is actually running, regardless of whether it goes on to win the lock itself.

Not documented as intended: docs/design.rst covers lock ownership for ticking, not for schedule installation.

Test

$ python -m unittest tests.test_issue_198 -v
test_setup_schedule_without_lock_leaves_schedule_alone ... expected failure
Ran 1 test in 0.012s
OK (expected failures=1)

Marked @unittest.expectedFailure so CI stays green. Whoever fixes this drops the marker and the test becomes the regression check — see the TRIAGE ARTIFACT docstring for where it should move (tests/test_scheduler.py).

Full suite unaffected: 90 tests, 2 pre-existing evalsha errors (missing fakeredis[lua] dev dependency, unrelated to this change), 2 skips, 1 expected failure (this one).

On the reporter's proposed InstallOnTickScheduler subclass

The shape is right, and gating tick() on the lock is doing necessary work rather than being belt-and-braces, since setup_schedule() runs from __init__ before self.lock can possibly be set — gating only there would mean it never runs at all. Two things worth considering for whoever picks this up:

  • Their tick() reruns the full install on every tick, not just the first one after acquisition. update_from_dict preserves last_run_at so it isn't destructive, but it is a rewrite of every static entry per tick; latching it (run once, on first lock acquisition) would avoid that.
  • The removal branch at the top of setup_schedule() — which deletes entries missing from this instance's config — is the sharp end and needs to be behind the lock in any fix.

Closes nothing; a maintainer decision on the actual fix approach is still needed.

RedBeatScheduler.__init__ runs setup_schedule() via the base Scheduler
constructor (non-lazy), which happens before beat_init fires and before
self.lock is ever set. A second instance started with a different
beat_schedule -- e.g. during a rolling release -- overwrites the statics
set the lock-holding instance is running, whether or not it goes on to
acquire the lock itself.

expectedFailure until the fix lands. TRIAGE ARTIFACT: move to
tests/test_scheduler.py at fix time, per the docstring.
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.

1 participant