Repository navigation
Completions sleep inside their transaction while holding locks #932
Description
Problem
Workflow task completions can sleep inside their database transaction while holding the task and run row locks.
At b2a33691:
- Service-operation retries.
start_service_operationrunsDefaultServiceControlPlane::execute(), whose retry loop (lines ~857–887) runs up to the catalog'smax_attemptsand callsusleep()for each backoff inside the completion transaction. - Synchronous update waits. A service operation's synchronous update wait polls with
usleep()(WorkflowStub, line ~2776) for a worker-chosenwait_timeout_secondsthat has no upper bound.
Every other writer of that run waits on those locks for the whole sleep.
Expected
- No sleeping or polling inside a completion transaction. Retry and wait move outside it into durable scheduling, or a bounded non-blocking check answers.
wait_timeout_secondshas a documented maximum and is refused above it.- A test shows a completion's transaction time is bounded independently of catalog backoff and worker wait values.
Found during the #930 audit (#930 (comment)). It is fixed separately from #930.
Activity
rmcdaniel commentedon Oct 25, 2026
General accepts ownership. The next implementation is a bounded, nonblocking or durably scheduled wait/retry path that does not sleep while the completion holds row locks, followed by measured transaction/lock controls. This remains separate from #930 and follows the #931 authority correction; no implementation or qualification is claimed.
rmcdaniel commentedon Oct 26, 2026
Current-source recovery on PR #936's accepted production tree confirms both lock-duration paths:
DefaultServiceControlPlane::dispatchHandler()performs the catalog retry loop synchronously and sleeps for every positive backoff. When invoked byDefaultWorkflowTaskBridge::applyStartServiceOperation(), the outer completion transaction still owns the task/run locks.WorkflowStub::waitForAcceptedUpdateToClose()normally polls after its own update-recording transaction. A synchronousWorkflowUpdateservice binding calls it from that same outer completion transaction, so the worker/catalogwait_timeout_secondsextends the completion lock hold.UpdateWaitPolicy::requestedTimeoutSeconds()currently accepts any positive integer.The narrow safe update boundary is clear: document and refuse requests above a fixed maximum (the existing contract already exercises 60 seconds), and force service-bound workflow updates to return at
acceptedwhenever an outer database transaction is open. Existing linked-call reconciliation/delivery can observe the durable update completion later.The retry half needs a real durable schedule rather than silently discarding backoff, serializing transient authority into a queue closure, or leaving a
retry_scheduledcall with no executor. The proposed contract is one idempotent attempt per claim of a durable service-call schedule: persist attempt/available-at state, enqueue only the call ID after commit, lock and revalidate the current caller run/call/catalog and stored accepted principal throughauthorizeReplay, execute outside caller-run locks, delay/reschedule from the catalog policy, and repair a lost post-commit enqueue. The completion transaction records only the scheduled call/event and bounded database work.Security contract review is requested before source changes. A narrower terminal first-attempt refusal is explicitly under consideration only if the durable scheduler scope is not required; no implementation or acceptance is claimed yet.
rmcdaniel commentedon Oct 26, 2026
Security design checkpoint — 2026-10-26 01:32 UTC.
Reviewed the current accepted PR #936 source at
fce3ac4b4448fc6b3835d338725a0d37d29d2af3, tree668c0f73abe5c801cc309dec75e4cbfb17de2f2f. The two issue paths are present as described: the service handler retry loop sleeps between attempts, and a service-bound workflow update can enter the synchronous polling wait while the outer completion transaction still owns its locks.Security accepts the durable scheduler as the intended #932 boundary. A terminal first-attempt/nonblocking refusal is not accepted as completion: it would change the accepted retry contract, discard catalog backoff and crash-recovery behavior, and substitute a product regression for the lock-duration fix.
Source acceptance conditions:
UpdateWaitPolicybefore coercion. Document and refuse values above 60 seconds at every ingress. When the shared storage connection is already in a transaction, choosewait_for=acceptedbefore enteringWorkflowStub; the returned surface must report the effective accepted-only behavior and must not claim that a completed wait occurred.ServiceOperationScheduled. It must not invoke a handler, poll, sleep, or enqueue before commit.caller_principal_*fields; callauthorizeReplay; and persist an attempt generation/claim fence plus the catalog/policy revision authorized at claim time. Accepted principal and scope fields must be immutable. Incomplete legacy authority fails closed.available_at, attempt/max-attempt, deadline, and retry state from the accepted retry policy. After-commit enqueue and a due-row repair scan must converge after a lost enqueue or crash. Duplicate and out-of-order jobs are harmless. Terminal caller/call, cancellation, and deadline state fence later attempts.The short claim transaction is the current-policy authorization point. The persisted claim revision and generation fence define the attempt permitted to execute after locks are released; that boundary should be explicit in the architecture documentation and tests.
rmcdaniel commentedon Oct 26, 2026
Implementation checkpoint — 2026-10-26 02:53 UTC
Draft stacked PR: #937
Exact source under review:
2a7511501bb91ba85e3ff250f65c14ae01ba4c93, tree876705cb83e81e1fe65896f60fa07f7fe441cc39, parent #936 headfce3ac4b4448fc6b3835d338725a0d37d29d2af3.The completion transaction now persists only the accepted service call, immutable accepted binding snapshot, durable dispatch row, and
ServiceOperationScheduledhistory. Queue publication is after commit and carries only the call ID. A short fixed-order claim transaction locks call → dispatch → current caller run → endpoint → service → operation, reconstructs the accepted principal, re-runs current replay authorization, and persists catalog/policy revisions plus generation/token fencing. The effect and retry backoff run outside every completion/claim transaction; due schedules, lost enqueue attempts, and expired claims are repairable.Additional acceptance edges are covered: all wait-time ingresses refuse above 60 seconds; authored synchronous waits remain blocked despite internal accepted-only dispatch; transient policy/storage failures roll back for repair rather than becoming authorization denials; final scheduled failures release admission only after current-claim CAS; and a cancellation requested after completion commit but before claim cancels the dispatch without creating a target.
Exact-source verification:
src testscleanThe disposable MySQL container and generated SQLite artifact were removed. Next action is Security exact-source review and terminal hosted CI for #937. This is not a merge or release request.
rmcdaniel commentedon Oct 26, 2026
The lock-duration correction is merged through #937 as ca4f04d, tree fd05be1c86c62a59c2a2214ed4cd675e048cecd9. Post-merge main build 40280106950 passed with 23 successful jobs, 2 inapplicable skips and 0 failures; main Public Boundary and CodeQL also passed.
Routine stable follow-through is now #938 at exact release-preparation head 881ad9e. This issue remains open until immutable 2.14.0 publication and published-package verification are recorded. No deployment has occurred.
rmcdaniel commentedon Oct 26, 2026
Workflow
2.14.0is published and its package-level acceptance is terminal.2026-10-26T10:53:46Zrefs/tags/2.14.0resolves directly to qualified commitb814c6c485b1c072f4092d3f478c7deba9c8c241(tree852928cf3924c63de6c88359d262783aee5940b4)durable-workflow/workflow2.14.0resolves both source and dist to that exact commitNo deployment is claimed. The owning issues remain open for downstream Server/package adoption and installed acceptance against the published artifact.
rmcdaniel commentedon Oct 26, 2026
The superseding stable adoption target is Workflow 2.14.1, not 2.14.0. It contains the full 2.14.0 train plus the external-worker pending-limit correction from #933.
The Rust Server owner has been told to keep its held 2.14.0 adoption unactivated and perform one adoption against 2.14.1. This issue remains open until its downstream installed acceptance is recorded.