You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: release SQL Server query locks after subscriber errors - #12939
A thrown/rejected beforeQuery hook exits after SQL Server's query lock is acquired but before its protected execution block. A rejected afterQuery promise similarly bypasses release() inside finally. Both leave subsequent queries on the runner waiting indefinitely.
Release and rethrow if pre-query logging/broadcasting fails; guarantee release with a nested finally around the post-query subscriber wait. Successful hook ordering, database error handling, and the awaited beforeQuery behavior introduced by #11086 are preserved. The fix adds no queries, timers, lock acquisitions, or successful-path asynchronous boundaries.
Validation (Node.js 22.17.1):
Three new cases fail on unchanged master: synchronous/asynchronous beforeQuery failures and asynchronous afterQuery failure block the next query.
All 7 lock tests pass after the fix, including synchronous afterQuery, pending-hook ordering, and a database-request error control.
The tests use the real SQL Server query runner, broadcaster and lock; only the mssql pool/request transport is stubbed. No live SQL Server instance was tested locally.
Full unit suite: 146 passing.
Complete TypeScript compilation, focused ESLint (no errors), formatting and diff checks pass.
Developed with AI assistance; source, prior fixes, regression behavior, and final diff reviewed.
Pull-Request Checklist
Code is up-to-date with the master branch
This pull request links a relevant issue using a closing keyword
There are new or updated tests validating the change
Documentation has been updated to reflect this change — N/A: restores failure cleanup without changing subscriber APIs
1. Later queries overlap unfinished hooks 🐞 Bug≡ Correctness
Description
The new beforeQuery catch releases the query lock as soon as broadcast() rejects, although its
promise wait can reject while another subscriber is still running. With one rejecting hook and
another pending hook, the next query acquires the lock before that pending hook settles; the new
afterQuery release path has the same behavior.
The broadcaster collects returned hook promises, but its wait uses fail-fast Promise.all. The
newly added release calls therefore unblock the next lock waiter on the first rejection rather than
after all started hooks settle; the lock test establishes that a pending hook should keep the next
query blocked.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
A rejected query hook can release the SQL Server query lock while other hooks from the same event remain pending, allowing the next query to overlap them.
## Fix Focus Areas
- packages/typeorm/src/driver/sqlserver/SqlServerQueryRunner.ts[228-233]
- packages/typeorm/src/driver/sqlserver/SqlServerQueryRunner.ts[355-359]
- packages/typeorm/src/subscriber/BroadcasterResult.ts[18-22]
## Recommended Fix
Ensure all started hook promises settle before either failure path releases the lock, then propagate the hook failure. Cover both before- and after-query events with a test containing one rejecting hook and one independently pending hook.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author self-review: I have reviewed the code review findings, and addressed the relevant ones.
Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cprakhar
left a comment
Description of change
Fixes #12936.
A thrown/rejected
beforeQueryhook exits after SQL Server's query lock is acquired but before its protected execution block. A rejectedafterQuerypromise similarly bypassesrelease()insidefinally. Both leave subsequent queries on the runner waiting indefinitely.Release and rethrow if pre-query logging/broadcasting fails; guarantee release with a nested
finallyaround the post-query subscriber wait. Successful hook ordering, database error handling, and the awaitedbeforeQuerybehavior introduced by #11086 are preserved. The fix adds no queries, timers, lock acquisitions, or successful-path asynchronous boundaries.Validation (Node.js 22.17.1):
beforeQueryfailures and asynchronousafterQueryfailure block the next query.afterQuery, pending-hook ordering, and a database-request error control.Developed with AI assistance; source, prior fixes, regression behavior, and final diff reviewed.
Pull-Request Checklist
masterbranch