Repository navigation
Avoid cross-run signal gap deadlocks in embedded workflow tasks - #1044
1 commit merged into
Conversation
rmcdaniel
left a comment
There was a problem hiding this comment.
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
RunWorkflowTasktransaction 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.
|
Workflow is taking repository-owner adoption and qualification for #1043 on exact candidate 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. |
|
Maintainer qualification now includes the complete existing Quality Cycle on combined source 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 The remaining source-compatibility requirement is MariaDB reached through Laravel's |
rmcdaniel
left a comment
There was a problem hiding this comment.
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.
e366443
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:
4cfa985b0aff1cef9b0f3df518b1d6c20023fa5cmain98bd98688663c005cb048d88f18119c35b6973b1The release and main have identical
src/,config/andcomposer.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
RunWorkflowTaskopensWorkflowSignalReadScopeafter it locks the task and run inside the root execution transaction.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.finallyon success or exception.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.
composer ecs -- --memory-limit=1G --no-progress-barcomposer stan -- --no-progress --memory-limit=1GCommands for the focused checks, after configuring a disposable MySQL database and isolated Redis:
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
PublicBoundaryCheckTest::testCommitMetadataScanAllowsBadTargetCommitReachableFromBaseline: the host's Bash 3.2 lacksmapfile. This is a recorded failed attempt, not a full-unit pass.The PR remains draft because the broader quality cycle is incomplete.
Compatibility
No schema, history event, codec, public API, serialized job payload,
$triesor workflow retry policy changes. No database-global isolation change or application-specific dependency is introduced.The optimized path uses Laravel's
mysqldriver check. MySQL 8.4.8 / REPEATABLE READ is qualified here. MariaDB configured through themysqldriver would also enter that path and has not been qualified by these local controls; the nativemariadbdriver 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.