Skip to content

Avoid cross-run signal gap deadlocks in embedded workflow tasks - #1044

Merged
1 commit merged into
durable-workflow:mainfrom
Loosand:codex/mysql-workflow-signal-gap-locks
Nov 5, 2026
Merged

1 commit merged into
durable-workflow:mainfrom
Loosand:codex/mysql-workflow-signal-gap-locks

Conversation

@Loosand

@Loosand Loosand commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

An embedded task continuing as new can deadlock with signal admission for another workflow instance on MySQL REPEATABLE READ. The terminal signal range locks and the signal command's history/task range locks form a cycle.

This candidate narrows signal locking inside the root embedded workflow-task transaction. It retains existing current locking reads for callers whose transaction/read view is not owned by that execution scope.

Addresses #1043. Maintainers will keep that issue open through published-package and affected-consumer acceptance. Related: #819 / #820 cover control-command rollback retries; #857 / #858 define the signal handoff and terminal disposition guarantees preserved here. #796 is a different watchdog/worker lock-order issue.

Latest-version verification

Checked on 2026-11-05:

Target Source Before-fix result
Latest release 2.20.3 4cfa985b0aff1cef9b0f3df518b1d6c20023fa5c Four real 1213 deadlocks / four concurrent controls
Current main 98bd98688663c005cb048d88f18119c35b6973b1 Four real 1213 deadlocks / four concurrent controls

The release and main have identical src/, config/ and composer.json. The release control ran in a separate tag-source checkout with only the feature test and fixture copied in. All 689 source files match the release tag, and runtime class/function bindings were checked. This verifies release-tag source, not a fresh Composer dist installation.

The precise workflow and before-fix instructions are in #1043. The tests control SQL interleaving with two PHP processes and independent PDO connections. Retry attempts are one, so a successful command retry cannot hide the cycle. The tests do not force a particular deadlock victim.

Implementation and consistency boundary

  • RunWorkflowTask opens WorkflowSignalReadScope after it locks the task and run inside the root execution transaction.
  • On a mysql-driver connection in that scope, signal handoff and terminal closure first discover candidate signals with a consistent read, then lock and recheck each candidate using its primary key plus the original predicates.
  • The scope is bound to the actual connection object, PDO and run ID. It is cleared in finally on success or exception.
  • Control commands, direct history recording, caller-owned outer transactions and other driver names retain their original range-locking current reads.

The consistent-read optimization relies on the embedded task acquiring its run lock before establishing its read view; ordinary signal admission uses that run lock too. A plain candidate read at every entry point would be unsafe: a caller can already have a REPEATABLE READ snapshot that predates an accepted signal. Transaction depth alone also fails to establish ownership because direct WorkflowHistoryEvent::record() adds a transaction level. The regression covers both cases.

Please review this ownership/read-view boundary in particular. The proposal preserves the signal integrity behavior from #858 while reducing the cross-run gap-lock footprint.

Verification

Candidate: 23c8fde91318df4327e62008f08c5e268d8c0166.

Environment: PHP 8.4.18, Laravel 13.40.0, Testbench 11.3.0, PHPUnit 13.4.6; native concurrency checks use MySQL 8.4.8 / REPEATABLE READ and isolated test services.

Check Result
Latest-release before-fix control 4 tests / 38 assertions / 4 expected failures, all actual 1213 deadlocks
Unmodified main before-fix control 4 tests / 38 assertions / 4 expected failures, all actual 1213 deadlocks
New native locking/read-view regression 12 tests / 92 assertions pass
Scope identity/cleanup unit controls 6 tests / 24 assertions pass
Combined native signal/continuation/command-retry regressions 52 tests / 676 assertions pass, including the 12 new native cases
Full ECS Pass: composer ecs -- --memory-limit=1G --no-progress-bar
Full PHPStan Pass: composer stan -- --no-progress --memory-limit=1G
Normal PR build Success on this exact head
Public Boundary Success on this exact head

Commands for the focused checks, after configuring a disposable MySQL database and isolated Redis:

vendor/bin/phpunit --testsuite feature --filter WorkflowTaskSignalGapLockTest
vendor/bin/phpunit tests/Unit/V2/WorkflowSignalReadScopeTest.php
composer test -- --filter 'TerminalSignalBoundaryTest|WorkflowTaskSignalGapLockTest|V2CommandDeadlockRetryTest|V2ContinueAsNewMetadataTest|V2MessageCursorContinueAsNewTest'

The combined checks cover immutable receipts, pending signal handoff, ordering, cold successor execution, terminal rejection, rollback, the prior control-command retry behavior, and default/named storage. The new caller-read-view cases cover cancellation, nested task continuation, direct terminal history recording and reuse after a failed task.

Checks not completed

  • The full local unit attempt stopped after 369 tests / 1,507 assertions with one failure in PublicBoundaryCheckTest::testCommitMetadataScanAllowsBadTargetCommitReachableFromBaseline: the host's Bash 3.2 lacks mapfile. This is a recorded failed attempt, not a full-unit pass.
  • Local coverage was not run: neither Xdebug nor PCOV is installed.
  • The earlier full feature attempt was interrupted when this submission was temporarily withdrawn. The 52 focused cases above have since completed.
  • The successful normal PR build is bounded PR CI. Its full database matrix, coverage and Sonar jobs were skipped by the existing workflow policy; those skips are not claimed as qualification.

The PR remains draft because the broader quality cycle is incomplete.

Compatibility

No schema, history event, codec, public API, serialized job payload, $tries or workflow retry policy changes. No database-global isolation change or application-specific dependency is introduced.

The optimized path uses Laravel's mysql driver check. MySQL 8.4.8 / REPEATABLE READ is qualified here. MariaDB configured through the mysql driver would also enter that path and has not been qualified by these local controls; the native mariadb driver retains the existing path. SERIALIZABLE and production load are not qualified.

Prepared with AI assistance. The recorded verification distinguishes observed results, source inspection and incomplete checks; implementation acceptance remains subject to review.

@Loosand Loosand closed this Nov 5, 2026
@rmcdaniel rmcdaniel added the intake:approved Current issue title and body revision is approved for authority intake label Nov 5, 2026
@Loosand Loosand reopened this Nov 5, 2026

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security exact-source review: PASS for 23c8fde91318df4327e62008f08c5e268d8c0166 / tree 72aa551294f1296e214c9a7952cef61adaec45fa over base 98bd98688663c005cb048d88f18119c35b6973b1.

I independently reproduced the predecessor failure on official MySQL 8.4.8: all four default/named history/task races reached the intended barrier once and failed the unrelated signal insert with SQLSTATE 40001 / error 1213. The exact head then passed 12 native cases plus six scope/identity unit cases (18 tests, 116 assertions) on the same tuple.

The source scope is appropriately narrow:

  • it opens the consistent-candidate path only inside the root MySQL RunWorkflowTask transaction after the exact run row is locked;
  • signal admission locks the workflow instance and resolved current run, so a same-run signal cannot commit between that run lock and the first candidate read;
  • each visible candidate is revalidated and locked by exact primary key before mutation;
  • the scope binds the Laravel connection object, raw PDO and run ID, restores a nested prior scope, and clears in finally;
  • caller-owned/nested transactions, older read views, other connections/drivers and direct terminal recording retain the existing locking read.

This approval is source-only. The reopened normal checks remain required, and merge/publication/downstream qualification remain with the owning maintainers. No release or runtime carry is inferred.

@Loosand
Loosand marked this pull request as ready for review November 5, 2026 10:44
@rmcdaniel

Copy link
Copy Markdown
Member

Workflow is taking repository-owner adoption and qualification for #1043 on exact candidate 23c8fde91318df4327e62008f08c5e268d8c0166, following General's priority decision. I reviewed the seven changed files and will preserve the contributor commit, author and regression coverage. I will not rewrite this contributor branch.

The existing independent Security baseline/candidate results and the actual hosted MySQL 12-case result remain exact-head evidence; they will be reused rather than presented as a new run. The full Quality Cycle and affected-package verification remain open, so #1043 stays open through those gates.

I am preparing an isolated qualification source reference that combines this product candidate with the separate test-schema correction #1040 and PostgreSQL timing correction #1041. Neither is a runtime dependency of the signal-locking fix; they address known full-QA harness failures. SQL #1030/#1037 and new #861 telemetry are excluded. General retains the exact full-run/integration decision; no new full run or release is started by this source preparation.

Any later contributor source commit will receive fresh source/effect review before credential-free disposable execution. The run boundary remains read-only reviewed source/vendor, temporary workspaces and synthetic local services, with no home, SSH, GitHub/API credentials, host configs or Docker socket forwarded.

@rmcdaniel

Copy link
Copy Markdown
Member

Maintainer qualification now includes the complete existing Quality Cycle on combined source 31aa05e42501a206bb2ce5d55b2fb7aeaa708201, preserving your exact commit 23c8fde91318df4327e62008f08c5e268d8c0166 and all seven file blobs. The two additional changes are the separately reviewed test-schema correction #1040 and PostgreSQL shard weights #1041. Your branch has not been rewritten.

Original full run 42018654305, attempt 1 succeeded: full style/static checks, 5,868 unit cases, all MySQL/PostgreSQL feature shards, configured MariaDB profiles, coverage ratchets and Laravel 10–13 upgrade journeys. All twelve new MySQL signal cases actually executed and passed. Exact counts, dependency graphs and skip attribution are recorded on #1043. Combined line coverage is 93.87%; the artifact's misleading main branch label is explicitly retained, with the immutable run/commit used as authority.

The remaining source-compatibility requirement is MariaDB reached through Laravel's mysql driver. The standard profile skips the new class on MariaDB, so we prepared a narrow twelve-case control for review. Maintainers own that execution and follow-through. Source integration and published-package acceptance remain pending; no additional full rerun is proposed.

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source approval for exact 23c8fde91318df4327e62008f08c5e268d8c0166. The root MySQL scope starts after the run/task locks and remains bound to the same connection, PDO and run. Candidate discovery is followed by exact primary-key predicate rechecks. Caller-owned transaction fallbacks and finally cleanup are preserved; no automatic replay of workflow code is added.

The preserved candidate plus #1040/#1041 passed the complete existing Quality Cycle in original run 42018654305/1, exact combined source 31aa05e4. The separate selected MariaDB-through-mysql control also passed all 12 tests / 92 assertions with zero errors, failures or skips; actual proof and independently observed cleanup are recorded. All seven contributor blobs remain exact and original authorship is preserved.

The explicit Laravel mariadb driver retains the existing path and is not qualified by this new-path control. Source integration remains General's selected sequence, and #1043 stays open for published-package and affected-consumer acceptance.

@rmcdaniel rmcdaniel closed this pull request by merging all changes into durable-workflow:main in e366443 Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

intake:approved Current issue title and body revision is approved for authority intake

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants