Do not revoke a lease that was renewed after it was listed as expired

[devin-ai-integration]

closes #22983

monitor_expired_leases lists the expired leases and schedules one revoke_expired_lease task for each lease. The holder can renew a lease before the task runs. The task read the lease again, but it did not check the expiration. Thus it decremented the slots and deleted a lease that was alive.

This PR adds a check of the current expiration in revoke_expired_lease. If the lease is no longer expired, the task stops and does not change the slots.

The memory lease storage kept the new expiration only in its expiration index. Therefore read_lease gave the old expiration and the new check could not see the renewal. renew_lease now also sets the expiration on the stored lease. The filesystem and Redis backends already do this.

Details

now = datetime.now(timezone.utc)
if expired_lease.expiration > now:
    logger.debug(f"Lease {lease_id} was renewed before revocation, skipping")
    return

Tests:

• tests/server/services/test_repossessor.py::TestRevokeExpiredLease::test_revoke_expired_lease_renewed_before_revocation — a renewed lease stays, and the active slots do not change. The test fails on main.
• tests/server/concurrency/test_memory_lease_storage.py::TestMemoryConcurrencyLeaseStorage::test_renew_lease_updates_lease_expiration — the lease expiration agrees with the expiration index after a renewal.

Checklist

• This pull request references any related issue by including "closes <link to issue>"
• If this is a complex change, a maintainer has confirmed the proposed approach on the linked issue.
  • The approach follows the fix proposed in the issue.
• If this pull request adds or changes functionality, it includes tests or explains why tests are not needed.
• If this pull request changes user-facing behavior, it updates documentation or explains why documentation is not needed.
  • This is a bug fix in server behavior; no documentation change is needed.
• If this pull request removes docs files, it includes redirect settings in mint.json.
• If this pull request adds functions or classes, it includes helpful docstrings.

Link to Devin session: https://app.devin.ai/sessions/b61c02f11a41438f81eea5a48a8f737b
Open in Devin Desktop: https://app.devin.ai/desktop/session/b61c02f11a41438f81eea5a48a8f737b?variant=devin

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

Devin Review found 2 potential issues.

[devin-ai-integration]

🔴 Renewal during revocation is lost

A renewal completing after read_lease remains invisible to this one-time expiration check. The task then deletes the renewed lease and releases its slots.

Prompt for agents

Close the remaining TOCTOU race in src/prefect/server/services/repossessor.py. revoke_expired_lease currently reads a lease, checks that snapshot's expiration, performs an asynchronous database decrement, and then unconditionally revokes the lease. A concurrent renew_lease call can succeed after the read/check and before revoke_lease, after which this task deletes the renewed lease and releases its slots. Add a storage-level conditional revocation or equivalent coordination that verifies the lease generation/expiration atomically with deletion, and ensure slot accounting remains consistent when that condition fails. Apply the contract across the memory, filesystem, and Redis storage implementations and add a deterministic test that renews after the task's expiration check but before deletion.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Correct, and intentionally out of scope here. A renewal that lands after this check but before revoke_lease is still lost, because none of the storage backends offer a conditional (compare-and-delete) revocation. Closing that needs a new contract on ConcurrencyLeaseStorage.revoke_lease (or a generation/expiration token) implemented in the memory, filesystem, and Redis backends, plus rollback of the slot decrement when the condition fails.

This PR shuts the window described in #22983 — the docket scheduling latency between listing and task start, which is the wide one. The residual window is the few milliseconds of the decrement itself. I would rather size the conditional-revocation change on its own issue than expand the API surface here.

[codspeed-hq]

Merging this PR will not alter performance

✅ 8 untouched benchmarks
⏩ 1 skipped benchmark¹

---

_{Comparing devin1/oss-8185-revoke_expired_lease-revokes-a-lease-that-was-renewed-after (8d4fb78) with main (33ccf3d)}

Footnotes

1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩