Skip to content

Reject expired acquisitions and cancel lost work-item leases - #586

Open
niemyjski wants to merge 6 commits into
feature/reject-lost-lock-renewalfrom
feature/pr580-review-hardening
Open

niemyjski wants to merge 6 commits into
feature/reject-lost-lock-renewalfrom
feature/pr580-review-hardening

Conversation

@niemyjski

@niemyjski niemyjski commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Stacked on #580. Cancel protected work immediately when work-item renewal reports lost ownership, reject leases that expire during slow collection acquisition, and await renewal shutdown before releasing the lease in the documentation example. The refreshed cache parent also rejects replacements that expire during preparation or retry.

Runtime behavior: lost work-item ownership cancels cooperative work sooner; expired acquisition sets return null or throw from AcquireAsync. Queue-only renewal errors remain logged. EmptyLock provides no exclusive protection, cancellation is not fencing, and provider timeouts remain necessary.

Validation at 3a888094addd3c8c0abefec38668dd7aa7a9e845: PR Build and push Build passed: 2,369 passed / 22 skipped / 0 failed. Local Release build, 194 conditional-cache cases, 67 lock/worker cases, formatting, and fresh binary APICompat passed. Existing ASPIRE010 warning remains.

Before merge/release: the child corrections must actually be incorporated into #580 before its dependent review findings can be closed. Combined/provider validation must be refreshed after the October 27 cache correction; older Redis results are historical. Durable provider registrations, non-Redis runtime limitations, next-release and documented performance/runtime acceptance remain separate requirements. No merge or release approval is inferred from green CI.

Verification and implementation details

Exact parent #580: 64a4fa6a68e42beff1d583c6763bc29035bf690a, including #570 0aa9d6804beefe88401eb7eddc0466afdc6d85ad. Main be10785cfed4e1850350dfaa024e1c3f25b7fac7 is an ancestor. The signed forward merge preserves both adjacent skill notes: immediate loss cancellation/acquisition latency and expired replacement rejection. Production code and tests merged cleanly.

Fresh binary API comparison: Microsoft.DotNet.ApiCompat 10.0.402 compared assemblies built from exact main be10785c against this final candidate. Foundatio net8.0/net10.0 and Foundatio.TestHarness net10.0 all exited 0 with no breaking changes, parameter-name checks enabled, and no suppressions. Binary compatibility is distinct from the intentional runtime behavior above.

Current local verification: Release build passed with zero errors and one existing ASPIRE010 warning. Conditional-cache selection: 194 passed; InMemoryLockTests plus WorkItemJobTests: 67 passed, no skips/failures. Changed-file dotnet format verification and git diff --check passed. Both full hosted suites executed 2,391 cases: 2,369 passed, 22 existing skips, zero failures.

The focused XML confirms these child regressions passed:

  • ProcessAsync_WhenAutoRenewedWorkItemLockIsLost_CancelsBeforeQueueRenewalCompletes: 2 cases, queue renewal eventually succeeds or fails; cancellation occurs before it completes, followed by abandonment.
  • ProcessAsync_WhenQueueRenewalFails_DoesNotCancelHandler: 1 case.
  • TryAcquireAsync_AfterSlowAcquisition_DoesNotReturnExpiredLease: 4 cases, one/two resources and 59/61-second acquisition latency.
  • TryAcquireAsync_AfterSlowRenewal_ReturnsOnlyUnexpiredLeases: 3 cases, live/expired leases and replacement ownership.

Historical boundary: October 23 combined verification at 70533664 included old #586 fffdf32f: core 2,484 passed/22 skipped, Redis 778 passed/18 skipped, and APICompat clean. All 30 lock and 12 added cache cases executed against matching binaries, using isolated Redis registrations. Those results are not relabeled as current provider proof after the October 27 correction. Durable provider-main/CI registration and non-Redis runtime coverage were not established by that run. The earlier all-provider source build also required an isolated RabbitMQ sample dependency overlay; raw unmodified provider-main success is not claimed. No combined stack or benchmark was rerun in this refresh.

Normal push CI publishes preview packages to GitHub Packages/Feedz; release publishing requires a version tag and was skipped. No workflow or security settings changed.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-28T04:27:07.670879Z
We couldn’t complete this review. Comment @codex review to try again.
3a88809 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@niemyjski niemyjski changed the title Cancel lost work-item leases before queue renewal completes Reject expired acquisitions and cancel lost work-item leases Oct 15, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

This branch has not been deployed

No deployments
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