Skip to content

feat(actor): add opt-in timer-discard shutdown policy - #488

Merged
dborgards merged 2 commits into
mainfrom
feat/actor-shutdown-policy-351
Oct 27, 2026
Merged

dborgards merged 2 commits into
mainfrom
feat/actor-shutdown-policy-351

Conversation

@dborgards

Copy link
Copy Markdown
Owner

What does this change?

Adds an explicit ActorShutdownPolicy.DiscardPendingTimers constructor option: finish accepted mailbox work while discarding pending timers, including already due ones. The original constructor and its DrainDueTimers default stay unchanged.

Timer selection and disposal share a short gate; callbacks and synchronization-context marshaling run outside it. A callback selected before disposal may still complete. The new policy does not complete tasks that depend on discarded timers; callers still need lifetime cancellation. XML docs, Actor README and ADR-6 describe the contract. The generated API approval adds only the enum and a three-argument constructor.

Closes #351. The prior minor fixes remain intact; this implements the newly authorized additive alternative rather than changing the legacy default.

Type of change

  • feat — new behaviour (minor release)
  • fix / perf
  • docs / test / refactor / chore / ci
  • Breaking change

Validation

  • 21 new deterministic cases across dedicated-thread, thread-pool and synchronization-context modes: original/explicit legacy behavior, reentrant/off-loop shutdown, due/future/cancelled timers, accepted/cancelled mailbox requests, pending inserts, invalid options and blocked context delivery.
  • Eight real source mutations failed at runtime and were restored: flipped default, ignored public option, retained shutdown timers, live-pass timer dispatch after stop, timers discarded while still live, accepted invalid options, lost accepted mailbox work, and holding the disposal gate across context dispatch.
  • Full local gates after the final test correction: 3125 net10 tests, net48 compile, CI build, format, nine verified packages, 50 engineering tests and traceability.
  • Final Cobertura checked across all 446 classes/nested types: 18 changed executable entries, no missing or partial coverage.
  • API approval copied from the actual generated received file after reviewing the additive diff. No real publication.

Checklist

  • Build with -p:CI=true, test (net10.0), dotnet format --verify-no-changes, and pack + eng/verify-packages.py all pass locally
  • Public API changes documented with XML comments
  • New behaviour covered by tests and source mutations
  • FR-RAW-020..024 / ADR-6 referenced

@cursor

cursor Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes concurrency-sensitive shutdown and timer firing in the protocol actor loop; default behavior is preserved, but incorrect ordering could affect teardown of ISO-TP/J1939-style protocol instances.

Overview
Adds ActorShutdownPolicy so ProtocolActor shutdown can optionally skip pending timers while still draining the mailbox. The existing two-parameter constructor is unchanged and keeps DrainDueTimers (run due timers after queued work; drop not-yet-due timers).

Callers can pass DiscardPendingTimers on a new three-argument constructor. FinalDrain then retires and clears the timer list instead of calling FireDueTimers. During live operation, timer dequeue is synchronized with disposal under _disposeGate so disposal does not select further timers under the opt-in policy, while a callback already removed from the queue may still run; user code and SynchronizationContext dispatch stay outside the lock.

Docs (Actor README, arc42 ADR-6) and API approval reflect the additive surface. New tests cover all execution modes, reentrant/off-loop shutdown, timers that become due mid-callback, pre-loop stop, and blocked context delivery.

Reviewed by Cursor Bugbot for commit 9f6441a. Bugbot is set up for automated code reviews on this repo. Configure here.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 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 ✅ Completed 2026-10-27T19:01:04.882925Z 9f6441a Manual request
ℹ️ 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.

Copy link
Copy Markdown
Owner Author

@codex review

Please review the current head 7d02828, including legacy-default compatibility, the disposal/timer-selection boundary and blocked synchronization-context dispatch.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 7d0282898f

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@codecov

codecov Bot commented Oct 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Owner Author

@codex review

Please review the current head 9f6441a after the test-only CodeQL762 using-ownership fix. The legacy/default and opt-in production implementations are unchanged.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 9f6441a8cc

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dborgards
dborgards merged commit 61d037a into main Oct 27, 2026
17 checks passed
@dborgards
dborgards deleted the feat/actor-shutdown-policy-351 branch October 27, 2026 19:31
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.

Actor/Reliability: minor findings and untested paths from the 2026-10-21 deep review (A-2..A-8)

2 participants