diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index db5f1c5da5..791de27b2e 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -1,6 +1,10 @@ { "name": "Microsoft.Data.SqlClient", - "dockerComposeFile": "docker-compose.yml", + "dockerComposeFile": [ + "docker-compose.yml", + "docker-compose.worktree.yml", + "docker-compose.local.yml" + ], "service": "devcontainer", "workspaceFolder": "/workspaces/SqlClient", "features": { @@ -20,6 +24,13 @@ } } }, + // Restore tools to enable compose file edits via powershell + "initializeCommand": "dotnet tool restore && dotnet tool run pwsh -- -NoProfile -ExecutionPolicy Bypass -File .devcontainer/init.ps1", "postCreateCommand": "bash .devcontainer/setup-sqlserver.sh", + // We need to mark the repository as safe because it's not owned by the docker user. + // Refreshing the git index now gets the container immediately ready for git operations, + // otherwise a git command needs to be manually run to refresh the index. + // Restore tools again within the container to ensure they are available for developer use + "postStartCommand": "git config --global --add safe.directory /workspaces/SqlClient && git update-index --really-refresh && dotnet tool restore", "remoteUser": "vscode" } diff --git a/.devcontainer/init.ps1 b/.devcontainer/init.ps1 new file mode 100644 index 0000000000..236f0bacff --- /dev/null +++ b/.devcontainer/init.ps1 @@ -0,0 +1,150 @@ +#!/usr/bin/env pwsh +#Requires -Version 7.0 +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +# ============================================================================= +# Cross-platform devcontainer initialization script. +# Runs on the HOST before the container starts. Detects worktree/project +# context and generates the .env file consumed by docker-compose.yml. +# +# Git worktree support: +# The .git file in a worktree contains an absolute host path. On Windows +# hosts this is a drive-letter path (C:/...) that Linux git cannot resolve. +# We maintain two path variants: +# - Host path: raw OS path used on the source side of bind mounts +# - Container path: POSIX path (/c/... on Windows) used inside the container +# A generated docker-compose.worktree.yml bind-mounts the main .git directory +# and overlays a corrected .git file so git works inside the container. +# +# Invoked via: dotnet tool run pwsh -- -NoProfile -ExecutionPolicy Bypass -File .devcontainer/init.ps1 +# ============================================================================= + +$devcontainerDir = $PSScriptRoot # .devcontainer/ + +# --- Worktree and project detection --- + +$worktreeDirName = Split-Path -Leaf (Get-Location) + +# Sanitize worktree name for use in DB names, container names, etc. +$worktreeName = ($worktreeDirName -replace '[^a-zA-Z0-9-]', '_').ToLowerInvariant() + +# Detect the current branch name. +$branchName = (git branch --show-current 2>$null) ?? '' +$branchName = ($branchName -replace '/', '-' -replace '[^a-zA-Z0-9-]', '_').ToLowerInvariant() + +# Resolve the git common directory (the main .git directory). +# In a worktree this points to the main repo's .git; in a standard clone it's just .git. +$gitCommonDir = git rev-parse --git-common-dir 2>$null +if (-not [System.IO.Path]::IsPathRooted($gitCommonDir)) { + $gitCommonDir = Join-Path (Get-Location) $gitCommonDir +} +$gitCommonDir = (Resolve-Path $gitCommonDir).Path + +# Host path: use forward slashes (Docker Desktop accepts both separators). +$gitCommonDirHost = $gitCommonDir -replace '\\', '/' + +# Container path: on Windows convert drive-letter paths ("C:/foo") to POSIX +# ("/c/foo") so they resolve inside the Linux container. +$gitCommonDirContainer = $gitCommonDirHost +if ($gitCommonDirContainer -match '^([A-Za-z]):(.*)$') { + $gitCommonDirContainer = '/' + $Matches[1].ToLowerInvariant() + $Matches[2] +} + +$mainRepoName = Split-Path -Leaf (Split-Path -Parent $gitCommonDirHost) +$projectName = $env:PROJECT_NAME ?? $mainRepoName + +$composeProjectName = "$projectName-$branchName" +$localWorkspaceFolder = (Get-Location).Path + +# Detect if this is a worktree (informational). +$dotGitPath = Join-Path (Get-Location) '.git' +if (Test-Path $dotGitPath -PathType Leaf) { + Write-Host "[devcontainer] Worktree detected. GIT_COMMON_DIR='$gitCommonDirHost' (container: '$gitCommonDirContainer')" +} else { + Write-Host "[devcontainer] Standard clone. GIT_COMMON_DIR='$gitCommonDirHost'" +} + +# --- Git worktree container fixup --- +# In a git worktree the .git *file* contains an absolute host path. +# Two problems must be solved for the Linux container: +# 1. On Windows hosts the path uses a drive letter (C:/...) that Linux git +# cannot resolve — rewrite it to the container-side POSIX path. +# 2. The main .git directory lives outside the workspace mount and must be +# bind-mounted into the container at the container-side path. +# +# We generate: +# .git-overlay – corrected .git file referencing container paths +# docker-compose.worktree.yml – bind-mounts the main .git dir + the overlay +# +# Long-form volume syntax is used for the GIT_COMMON_DIR mount so Docker does +# not try to split the path on colons (which breaks Windows drive letters). +# +# For standard (non-worktree) clones both files are no-op stubs. + +$worktreeComposePath = Join-Path $devcontainerDir 'docker-compose.worktree.yml' +$gitOverlayPath = Join-Path $devcontainerDir '.git-overlay' + +if (Test-Path $dotGitPath -PathType Leaf) { + # Worktree: .git is a file — read it and rewrite with container-side paths. + $gitFileContent = (Get-Content $dotGitPath -Raw).Trim() + if ($gitFileContent -match 'gitdir:\s*(.+)') { + $originalGitdir = $Matches[1].Trim() -replace '\\', '/' + # Convert Windows drive-letter path to POSIX for the container. + $containerGitdir = $originalGitdir + if ($containerGitdir -match '^([A-Za-z]):(.*)$') { + $containerGitdir = '/' + $Matches[1].ToLowerInvariant() + $Matches[2] + } + Set-Content -Path $gitOverlayPath -Value "gitdir: $containerGitdir`n" -NoNewline + + # Generate compose override. Long-form volume for the GIT_COMMON_DIR + # mount avoids colon-parsing issues with Windows paths. + $worktreeComposeContent = @" +# Auto-generated by init.ps1 - DO NOT EDIT. +# Bind-mounts the main .git directory and overlays a corrected .git file +# so that git works inside the container for worktree checkouts. +services: + devcontainer: + volumes: + - type: bind + source: $gitCommonDirHost + target: $gitCommonDirContainer + - ./.git-overlay:/workspaces/SqlClient/.git:ro +"@ + Set-Content -Path $worktreeComposePath -Value $worktreeComposeContent -NoNewline + Write-Host "[devcontainer] Generated git worktree overlay for container." + } else { + Write-Host "[devcontainer] WARNING: Could not parse .git file at '$dotGitPath'." + Set-Content -Path $worktreeComposePath -Value "# Auto-generated by init.ps1 - could not parse .git file.`nservices: {}" -NoNewline + if (Test-Path $gitOverlayPath) { Remove-Item $gitOverlayPath } + } +} else { + # Standard clone: .git is a directory, already covered by the workspace mount. + Set-Content -Path $worktreeComposePath -Value "# Auto-generated by init.ps1 - standard clone, no worktree fixup needed.`nservices: {}" -NoNewline + if (Test-Path $gitOverlayPath) { Remove-Item $gitOverlayPath } +} + +# --- Local compose overrides --- +# docker-compose.local.yml is listed in devcontainer.json so it must exist. +# Create an empty stub if missing (gitignored — developers can add personal overrides). + +$localComposePath = Join-Path $devcontainerDir 'docker-compose.local.yml' +if (-not (Test-Path $localComposePath)) { + Set-Content -Path $localComposePath -Value "# Personal Docker Compose overrides (gitignored).`n# See docker-compose.yml for the base configuration.`n" +} + +# --- Write .env for docker-compose variable substitution --- + +$envContent = @" +COMPOSE_PROJECT_NAME=$composeProjectName +WORKTREE_NAME=$worktreeName +BRANCH_NAME=$branchName +GIT_COMMON_DIR_HOST=$gitCommonDirHost +GIT_COMMON_DIR_CONTAINER=$gitCommonDirContainer +MAIN_REPO_NAME=$mainRepoName +PROJECT_NAME=$projectName +LOCAL_WORKSPACE_FOLDER=$localWorkspaceFolder +"@ +Set-Content -Path (Join-Path $devcontainerDir '.env') -Value $envContent -NoNewline + +Write-Host "[devcontainer] init.ps1 complete for worktree '$worktreeName' branch '$branchName' (project: $projectName)" diff --git a/.devcontainer/setup-sqlserver.sh b/.devcontainer/setup-sqlserver.sh old mode 100755 new mode 100644 diff --git a/.devcontainer/sqlserver-entrypoint.sh b/.devcontainer/sqlserver-entrypoint.sh old mode 100755 new mode 100644 diff --git a/.gitattributes b/.gitattributes index 1ff0c42304..0bb844340a 100644 --- a/.gitattributes +++ b/.gitattributes @@ -3,6 +3,10 @@ ############################################################################### * text=auto +# Shell scripts must always use LF line endings, even on Windows checkouts, +# because they are bind-mounted into Linux containers. +*.sh text eol=lf + ############################################################################### # Set default behavior for command prompt diff. # diff --git a/.github/prompts/generate-doc-comments.prompt.md b/.github/prompts/generate-doc-comments.prompt.md index 33ced936de..9361d3c7ad 100644 --- a/.github/prompts/generate-doc-comments.prompt.md +++ b/.github/prompts/generate-doc-comments.prompt.md @@ -1,5 +1,5 @@ --- -name: doc-comments +name: generate-doc-comments description: Generate XML documentation comments for C# code following .NET best practices. argument-hint: agent: agent diff --git a/.github/prompts/generate-prompt.prompt.md b/.github/prompts/generate-prompt.prompt.md index a4aa8cd40f..73d083947b 100644 --- a/.github/prompts/generate-prompt.prompt.md +++ b/.github/prompts/generate-prompt.prompt.md @@ -34,6 +34,7 @@ Before generating the prompt, review the available skills in the `.github/skills * `name`: A concise, kebab-case name for the prompt. * `description`: A clear, short description of what the prompt does. * `argument-hint`: (Optional) A hint for what arguments the user can provide when using the prompt. + * `tools`: (Recommended) A list of tool identifiers the prompt is allowed to use. See the **Tool Scoping** section below. * **Body Structure**: * **Role**: Define the AI's persona (e.g., "You are an expert C# developer..."). * **Context**: Include specific context instructions or references. @@ -50,12 +51,70 @@ Before generating the prompt, review the available skills in the `.github/skills * Use `${input:variableName}` for user inputs (e.g., `${input:methodName}`). * Use built-in variables like `${selection}`, `${file}`, or `${workspaceFolder}` where appropriate context is needed. -6. **Best Practices**: +6. **Scope Tools**: Restrict the tools available to each prompt using the `tools` frontmatter field. See the **Tool Scoping** section below for detailed guidance. + +7. **Best Practices**: * Be specific and explicit. * Encourage chain-of-thought reasoning if the task is complex. * Reference workspace files using Markdown links `[path/to/file.cs](path/to/file.cs)` only if they are static and necessary for *all* invocations of this prompt. * Prefer referencing skills over duplicating instructions that already exist in skills. +## Tool Scoping + +Every generated prompt **should** include a `tools` list in its YAML frontmatter. Scoping tools keeps the model focused by limiting it to approved, known-effective tools for the task. Without tool scoping, the model may invoke irrelevant tools, waste context, or produce unpredictable results. + +### Why scope tools? +- **Focus**: Fewer tools means the model spends less reasoning on tool selection and more on the task. +- **Reliability**: Restricting to tested tools avoids unexpected side effects (e.g., a read-only review prompt shouldn't have edit tools). +- **Safety**: Prevents prompts from accidentally running terminal commands or making file changes when they shouldn't. + +### How to choose tools +Apply the **principle of least privilege** — include only the tools the prompt actually needs: + +| Prompt type | Recommended tools | +|---|---| +| **Read-only analysis** (review, triage, explain) | `read/readFile`, `search/codebase`, `search/textSearch` | +| **Code editing** (bug fix, feature, refactor) | `edit/editFiles`, `edit/createFile`, `read/readFile`, `search/codebase` | +| **Needs terminal** (build, test, scripts) | All of the above + `execute/runInTerminal`, `execute/getTerminalOutput` | +| **Needs GitHub data** (triage, release notes) | All of the above + `github/search_issues` or other GitHub tools | +| **Needs web content** (docs lookup) | `web/fetch` | + +### Available built-in tool identifiers + +You can specify individual tools or tool sets (which include all tools in that group). + +**Tool sets** (use these to include all tools in a category): +- `edit` — File creation and editing tools +- `read` — File and notebook reading tools +- `search` — Codebase, text, and file search tools +- `execute` — Terminal, task, and notebook execution tools +- `web` — Web content fetching tools + +**Commonly used individual tools:** + +| Tool identifier | Purpose | +|---|---| +| `edit/editFiles` | Apply edits to existing files | +| `edit/createFile` | Create a new file | +| `read/readFile` | Read file contents | +| `read/problems` | Get workspace problems/diagnostics | +| `search/codebase` | Semantic code search | +| `search/textSearch` | Text/regex search in files | +| `search/fileSearch` | Search for files by glob pattern | +| `search/listDirectory` | List directory contents | +| `search/usages` | Find references and implementations | +| `execute/runInTerminal` | Run a shell command | +| `execute/getTerminalOutput` | Get terminal output | +| `execute/testFailure` | Get test failure details | +| `web/fetch` | Fetch a web page | + +**Extension / MCP tools** can also be included using their identifier (e.g., `github/search_issues`). Use `/*` to include all tools from an MCP server. + +### Frontmatter syntax +```yaml +tools: ['read/readFile', 'search/codebase', 'edit/editFiles'] +``` + ## Example Output Structure (with skill reference) ```markdown @@ -63,6 +122,7 @@ Before generating the prompt, review the available skills in the `.github/skills name: my-new-prompt description: specialized task description argument-hint: input parameter hint +tools: ['edit/editFiles', 'read/readFile', 'search/codebase', 'execute/runInTerminal'] --- You are a specialized agent for... @@ -89,6 +149,7 @@ Use ${input:param1} to... name: my-new-prompt description: specialized task description argument-hint: input parameter hint +tools: ['read/readFile', 'search/codebase'] --- You are a specialized agent for... diff --git a/.github/prompts/refine-test-overlap.prompt.md b/.github/prompts/refine-test-overlap.prompt.md index 824e18f145..f4d0339c27 100644 --- a/.github/prompts/refine-test-overlap.prompt.md +++ b/.github/prompts/refine-test-overlap.prompt.md @@ -1,7 +1,9 @@ --- -name: test-minimize-overlap +name: refine-test-overlap description: Run coverage overlap analysis and suggest test suite optimizations argument-hint: Test filter (e.g. FullyQualifiedName~MyTests) or describe the tests you want to analyze +agent: agent +tools: ['edit/editFiles', 'read/readFile', 'search/codebase', 'execute/runInTerminal', 'execute/getTerminalOutput'] --- You are an expert .NET Test Engineer specialized in optimizing test coverage and reducing technical debt. @@ -10,19 +12,19 @@ Your task is to analyze the user's test suite using the `AnalyzeTestOverlap.ps1` ## Skills This prompt leverages the following skills for specific sub-tasks: -- [generate-mstest-filter](../skills/generate-mstest-filter/SKILL.md) - For generating well-formed MSTest filter expressions +- [generate-mstest-filter](.github/skills/generate-mstest-filter/SKILL.md) - For generating well-formed MSTest filter expressions ## Tools -You have access to the analysis script at `[AnalyzeTestOverlap.ps1](./scripts/AnalyzeTestOverlap.ps1)`. +You have access to the analysis script at [AnalyzeTestOverlap.ps1](.github/prompts/scripts/AnalyzeTestOverlap.ps1). ## Workflow 1. **Parse or Generate Test Filter**: * If `${input:filter}` is a valid MSTest filter expression (e.g., `FullyQualifiedName~MyTests`), use it directly. - * If `${input:filter}` is a loose description (e.g., "connection tests" or "SqlCommand class"), follow the instructions in the [generate-mstest-filter](../skills/generate-mstest-filter/SKILL.md) skill to generate a proper filter expression. + * If `${input:filter}` is a loose description (e.g., "connection tests" or "SqlCommand class"), follow the instructions in the [generate-mstest-filter](.github/skills/generate-mstest-filter/SKILL.md) skill to generate a proper filter expression. * If `${input:filter}` is empty, ask the user for a test filter or description to target specific tests. 2. **Run Analysis**: - * Run the script using the filter: `.\scripts\AnalyzeTestOverlap.ps1 -Filter ""`. + * Run the script from the workspace root: `.\.github\prompts\scripts\AnalyzeTestOverlap.ps1 -Filter ""`. * *Note*: The script produces a console summary and a `test-coverage-analysis.json` file. 3. **Review Overlap**: diff --git a/.gitignore b/.gitignore index 0d8dfed6d5..156fc21e11 100644 --- a/.gitignore +++ b/.gitignore @@ -376,3 +376,9 @@ MigrationBackup/ # MDS "Not Supported" GenAPI code **/notsupported/*.cs + +# Devcontainer generated files (created by init.ps1) +.devcontainer/.env +.devcontainer/.git-overlay +.devcontainer/docker-compose.local.yml +.devcontainer/docker-compose.worktree.yml diff --git a/dotnet-tools.json b/dotnet-tools.json index 1f59e06063..11e3494512 100644 --- a/dotnet-tools.json +++ b/dotnet-tools.json @@ -15,6 +15,13 @@ "apicompat" ], "rollForward": false + }, + "powershell": { + "version": "7.6.0", + "commands": [ + "pwsh" + ], + "rollForward": false } } } \ No newline at end of file diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index 63815e8200..553e6cd395 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -96,6 +96,7 @@ internal ChannelDbConnectionPool( Identity = identity; AuthenticationContexts = new(); MaxPoolSize = Convert.ToUInt32(PoolGroupOptions.MaxPoolSize); + TransactedConnectionPool = new(this); _connectionSlots = new(MaxPoolSize); @@ -148,6 +149,9 @@ public ConcurrentDictionary< /// public DbConnectionPoolState State { get; private set; } + /// + public TransactedConnectionPool TransactedConnectionPool { get; } + /// public bool UseLoadBalancing => PoolGroupOptions.UseLoadBalancing; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolOptions.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolOptions.cs index 7adf8abdc2..5ac6f4d565 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolOptions.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolOptions.cs @@ -39,6 +39,9 @@ bool hasTransactionAffinity _hasTransactionAffinity = hasTransactionAffinity; } + /// + /// The time (in milliseconds) to wait for a connection to be created/returned before terminating the attempt. + /// public int CreationTimeout { get { return _creationTimeout; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs index b684bb24bb..bfc5789d3f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs @@ -85,6 +85,11 @@ internal interface IDbConnectionPool /// DbConnectionPoolState State { get; } + /// + /// Holds connections that are currently enlisted in a transaction. + /// + TransactedConnectionPool TransactedConnectionPool { get; } + /// /// Indicates whether the connection pool is using load balancing. /// @@ -106,7 +111,7 @@ internal interface IDbConnectionPool /// The user options to use if a new connection must be opened. /// The retrieved connection will be passed out via this parameter. /// True if a connection was set in the out parameter, otherwise returns false. - bool TryGetConnection(DbConnection owningObject, TaskCompletionSource taskCompletionSource, DbConnectionOptions userOptions, out DbConnectionInternal? connection); + bool TryGetConnection(DbConnection owningObject, TaskCompletionSource? taskCompletionSource, DbConnectionOptions userOptions, out DbConnectionInternal? connection); /// /// Replaces the internal connection currently associated with owningObject with a new internal connection from the pool. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/TransactedConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/TransactedConnectionPool.cs index 7245164f2c..3ce203c071 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/TransactedConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/TransactedConnectionPool.cs @@ -31,7 +31,7 @@ internal class TransactedConnectionPool /// A specialized list that holds database connections associated with a specific transaction. /// Maintains a reference to the transaction for proper cleanup when the transaction completes. /// - private sealed class TransactedConnectionList : List + internal sealed class TransactedConnectionList : List { private readonly Transaction _transaction; @@ -60,9 +60,6 @@ internal void Dispose() } #region Fields - - private readonly Dictionary _transactedCxns; - private static int _objectTypeCount; internal readonly int _objectID = System.Threading.Interlocked.Increment(ref _objectTypeCount); @@ -80,7 +77,7 @@ internal void Dispose() internal TransactedConnectionPool(IDbConnectionPool pool) { Pool = pool; - _transactedCxns = new Dictionary(); + TransactedConnections = new Dictionary(); SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Constructed for connection pool {1}", Id, Pool.Id); } @@ -98,6 +95,8 @@ internal TransactedConnectionPool(IDbConnectionPool pool) /// The IDbConnectionPool instance that owns this transacted pool. internal IDbConnectionPool Pool { get; } + internal Dictionary TransactedConnections { get; } + #endregion #region Methods @@ -123,9 +122,9 @@ internal TransactedConnectionPool(IDbConnectionPool pool) TransactedConnectionList? connections; bool txnFound = false; - lock (_transactedCxns) + lock (TransactedConnections) { - txnFound = _transactedCxns.TryGetValue(transaction, out connections); + txnFound = TransactedConnections.TryGetValue(transaction, out connections); } // NOTE: GetTransactedObject is only used when AutoEnlist = True and the ambient transaction @@ -182,10 +181,10 @@ internal void PutTransactedObject(Transaction transaction, DbConnectionInternal // NOTE: because TransactionEnded is an asynchronous notification, there's no guarantee // around the order in which PutTransactionObject and TransactionEnded are called. - lock (_transactedCxns) + lock (TransactedConnections) { // Check if a transacted pool has been created for this transaction - if ((txnFound = _transactedCxns.TryGetValue(transaction, out connections)) + if ((txnFound = TransactedConnections.TryGetValue(transaction, out connections)) && connections is not null) { // synchronize multi-threaded access with GetTransactedObject @@ -213,14 +212,14 @@ internal void PutTransactedObject(Transaction transaction, DbConnectionInternal transactionClone = transaction.Clone(); newConnections = new TransactedConnectionList(2, transactionClone); // start with only two connections in the list; most times we won't need that many. - lock (_transactedCxns) + lock (TransactedConnections) { // NOTE: in the interim between the locks on the transacted pool (this) during // execution of this method, another thread (threadB) may have attempted to // add a different connection to the transacted pool under the same // transaction. As a result, threadB may have completed creating the // transacted pool while threadA was processing the above instructions. - if (_transactedCxns.TryGetValue(transaction, out connections) + if (TransactedConnections.TryGetValue(transaction, out connections) && connections is not null) { // synchronize multi-threaded access with GetTransactedObject @@ -238,7 +237,7 @@ internal void PutTransactedObject(Transaction transaction, DbConnectionInternal // add the connection/transacted object to the list newConnections.Add(transactedObject); - _transactedCxns.Add(transactionClone, newConnections); + TransactedConnections.Add(transactionClone, newConnections); transactionClone = null; // we've used it -- don't throw it or the TransactedConnectionList that references it away. } } @@ -297,9 +296,9 @@ internal void TransactionEnded(Transaction transaction, DbConnectionInternal tra // TODO: that the pending creation of a transacted pool for this transaction is aborted when // TODO: PutTransactedObject finally gets some CPU time? - lock (_transactedCxns) + lock (TransactedConnections) { - if (_transactedCxns.TryGetValue(transaction, out connections) + if (TransactedConnections.TryGetValue(transaction, out connections) && connections is not null) { bool shouldDisposeConnections = false; @@ -319,7 +318,7 @@ internal void TransactionEnded(Transaction transaction, DbConnectionInternal tra if (0 >= connections.Count) { SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction {1}, Removing List from transacted pool.", Id, transaction.GetHashCode()); - _transactedCxns.Remove(transaction); + TransactedConnections.Remove(transaction); // we really need to dispose our connection list; it may have // native resources via the tx and GC may not happen soon enough. @@ -351,4 +350,4 @@ internal void TransactionEnded(Transaction transaction, DbConnectionInternal tra } #endregion -} \ No newline at end of file +} diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs index 194054e58e..8d2311b5cd 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs @@ -305,9 +305,9 @@ public bool IsRunning get { return State is Running; } } - private int MaxPoolSize => PoolGroupOptions.MaxPoolSize; + internal int MaxPoolSize => PoolGroupOptions.MaxPoolSize; - private int MinPoolSize => PoolGroupOptions.MinPoolSize; + internal int MinPoolSize => PoolGroupOptions.MinPoolSize; public DbConnectionPoolGroup PoolGroup => _connectionPoolGroup; @@ -324,6 +324,8 @@ public bool IsRunning private bool UsingIntegrateSecurity => _identity != null && DbConnectionPoolIdentity.NoIdentity != _identity; + public TransactedConnectionPool TransactedConnectionPool => _transactedConnectionPool; + private void CleanupCallback(object state) { // Called when the cleanup-timer ticks over. @@ -940,6 +942,8 @@ private bool TryGetConnection(DbConnection owningObject, uint waitForMultipleObj // If automatic transaction enlistment is required, then we try to // get the connection from the transacted connection pool first. + // If automatic enlistment is not enabled, then we cannot vend connections + // from the transacted pool. if (HasTransactionAffinity) { obj = GetFromTransactedPool(out transaction); diff --git a/src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Tests/SqlClientTestGroup.cs b/src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Tests/SqlClientTestGroup.cs index e48c13d49b..7c7597ba8a 100644 --- a/src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Tests/SqlClientTestGroup.cs +++ b/src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Tests/SqlClientTestGroup.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using System.Diagnostics; using System.Data; +using System.Transactions; using Microsoft.Data.SqlClient; using System.Xml; diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs index 3fc31338cd..18bd9c5ea3 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs @@ -668,13 +668,14 @@ internal class MockDbConnectionPool : IDbConnectionPool public DbConnectionPoolGroupOptions PoolGroupOptions => throw new NotImplementedException(); public DbConnectionPoolProviderInfo ProviderInfo => throw new NotImplementedException(); public DbConnectionPoolState State => throw new NotImplementedException(); + public TransactedConnectionPool TransactedConnectionPool => throw new NotImplementedException(); public bool UseLoadBalancing => throw new NotImplementedException(); public ConcurrentBag ReturnedConnections { get; } = new(); public void Clear() => throw new NotImplementedException(); - public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource taskCompletionSource, DbConnectionOptions userOptions, out DbConnectionInternal? connection) + public bool TryGetConnection(DbConnection owningObject, TaskCompletionSource? taskCompletionSource, DbConnectionOptions userOptions, out DbConnectionInternal? connection) { throw new NotImplementedException(); } @@ -739,4 +740,4 @@ internal override void ResetConnection() } #endregion -} \ No newline at end of file +} diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs new file mode 100644 index 0000000000..cafea0dad4 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs @@ -0,0 +1,951 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Data; +using System.Data.Common; +using System.Threading; +using System.Threading.Tasks; +using System.Transactions; +using Microsoft.Data.Common.ConnectionString; +using Microsoft.Data.ProviderBase; +using Microsoft.Data.SqlClient.ConnectionPool; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests.ConnectionPool; + +/// +/// Deterministic tests for WaitHandleDbConnectionPool transaction functionality. +/// These tests exercise transacted connection pathways with controlled synchronization +/// to verify correct behavior without relying on probabilistic concurrency. +/// +public class WaitHandleDbConnectionPoolTransactionTest : IDisposable +{ + private const int DefaultMaxPoolSize = 50; + private const int DefaultMinPoolSize = 0; + private const int DefaultCreationTimeoutInMilliseconds = 15000; + + private IDbConnectionPool _pool = null!; + + public WaitHandleDbConnectionPoolTransactionTest() + { + _pool = CreatePool(); + } + + public void Dispose() + { + // Verify no leaked transactions before cleanup + Assert.Empty(_pool.TransactedConnectionPool.TransactedConnections); + + _pool?.Shutdown(); + _pool?.Clear(); + } + + #region Helper Methods + + private WaitHandleDbConnectionPool CreatePool( + int maxPoolSize = DefaultMaxPoolSize, + int minPoolSize = DefaultMinPoolSize, + bool hasTransactionAffinity = true) + { + var poolGroupOptions = new DbConnectionPoolGroupOptions( + poolByIdentity: false, + minPoolSize: minPoolSize, + maxPoolSize: maxPoolSize, + creationTimeout: DefaultCreationTimeoutInMilliseconds, + loadBalanceTimeout: 0, + hasTransactionAffinity: hasTransactionAffinity + ); + + var dbConnectionPoolGroup = new DbConnectionPoolGroup( + new DbConnectionOptions("DataSource=localhost;", null), + new DbConnectionPoolKey("TestDataSource"), + poolGroupOptions + ); + + var connectionFactory = new MockSqlConnectionFactory(); + + var pool = new WaitHandleDbConnectionPool( + connectionFactory, + dbConnectionPoolGroup, + DbConnectionPoolIdentity.NoIdentity, + new DbConnectionPoolProviderInfo() + ); + + pool.Startup(); + return pool; + } + + private DbConnectionInternal GetConnection(SqlConnection owner) + { + _pool.TryGetConnection( + owner, + taskCompletionSource: null, + new DbConnectionOptions("", null), + out DbConnectionInternal? connection); + return connection!; + } + + private async Task GetConnectionAsync( + SqlConnection owner, + Transaction? transaction = null) + { + var tcs = new TaskCompletionSource(transaction); + _pool.TryGetConnection( + owner, + taskCompletionSource: tcs, + new DbConnectionOptions("", null), + out DbConnectionInternal? connection); + return connection ?? await tcs.Task; + } + + private void ReturnConnection(DbConnectionInternal connection, SqlConnection owner) + { + _pool.ReturnInternalConnection(connection, owner); + } + + private void AssertPoolMetrics() + { + Assert.True(_pool.Count <= _pool.PoolGroupOptions.MaxPoolSize, + $"Pool count ({_pool.Count}) exceeded max pool size ({_pool.PoolGroupOptions.MaxPoolSize})"); + Assert.True(_pool.Count >= 0, + $"Pool count ({_pool.Count}) is negative"); + Assert.Empty(_pool.TransactedConnectionPool.TransactedConnections); + } + + #endregion + + #region Transaction Routing Tests + + [Fact] + public void GetConnection_UnderTransaction_RoutesToTransactedPool() + { + // Arrange & Act + using var scope = new TransactionScope(); + var transaction = Transaction.Current; + Assert.NotNull(transaction); + + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + + ReturnConnection(conn, owner); + + // Assert - connection should be in the transacted pool + Assert.True(_pool.TransactedConnectionPool.TransactedConnections.ContainsKey(transaction)); + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections[transaction]); + + scope.Complete(); + } + + [Fact] + public void GetConnection_WithoutTransaction_RoutesToGeneralPool() + { + // Arrange & Act (no TransactionScope) + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + + ReturnConnection(conn, owner); + + // Assert - transacted pool should be empty + Assert.Empty(_pool.TransactedConnectionPool.TransactedConnections); + } + + [Fact] + public void GetConnection_UnderTransaction_ReturnsSameConnectionFromTransactedPool() + { + // Arrange + using var scope = new TransactionScope(); + + // Act - first call creates a new connection + var owner1 = new SqlConnection(); + var conn1 = GetConnection(owner1); + Assert.NotNull(conn1); + ReturnConnection(conn1, owner1); + + // Second call should retrieve the SAME connection from the transacted pool (LIFO) + var owner2 = new SqlConnection(); + var conn2 = GetConnection(owner2); + Assert.NotNull(conn2); + Assert.Same(conn1, conn2); + + ReturnConnection(conn2, owner2); + scope.Complete(); + } + + [Fact] + public async Task GetConnectionAsync_UnderTransaction_ReturnsSameConnectionFromTransactedPool() + { + // Arrange + using var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled); + var transaction = Transaction.Current; + + // Act - first call creates a new connection + var owner1 = new SqlConnection(); + var conn1 = await GetConnectionAsync(owner1, transaction: transaction); + Assert.NotNull(conn1); + ReturnConnection(conn1, owner1); + + // Second call should retrieve the SAME connection from the transacted pool + var owner2 = new SqlConnection(); + var conn2 = await GetConnectionAsync(owner2, transaction: transaction); + Assert.NotNull(conn2); + Assert.Same(conn1, conn2); + + ReturnConnection(conn2, owner2); + scope.Complete(); + } + + [Fact] + public void GetConnection_WithTransactionAffinityDisabled_SkipsTransactedPool() + { + // Arrange + _pool.Shutdown(); + _pool.Clear(); + _pool = CreatePool(hasTransactionAffinity: false); + + using var scope = new TransactionScope(); + + // Act + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + + // Assert - even though a transaction is active, transacted pool is not used + Assert.Empty(_pool.TransactedConnectionPool.TransactedConnections); + + scope.Complete(); + } + + #endregion + + #region Transaction Lifecycle Tests + + [Fact] + public void TransactionCommit_ClearsTransactedPool() + { + // Arrange & Act + using (var scope = new TransactionScope()) + { + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + + // While transaction is active, connection should be in transacted pool + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + + scope.Complete(); + } + + // Assert - after transaction completes, transacted pool should be empty + AssertPoolMetrics(); + } + + [Fact] + public void TransactionRollback_ClearsTransactedPool() + { + // Arrange & Act + using (var scope = new TransactionScope()) + { + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + + // Don't call scope.Complete() — triggers rollback + } + + // Assert - transacted pool should be empty after rollback too + AssertPoolMetrics(); + } + + [Fact] + public void MultipleGetReturn_SameTransaction_ReusesConnection() + { + // Arrange + using var scope = new TransactionScope(); + var transaction = Transaction.Current; + Assert.NotNull(transaction); + + // Act - get and return multiple times within same transaction + for (int i = 0; i < 10; i++) + { + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + } + + // Assert - only one connection should be in the transacted pool + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections[transaction]); + + scope.Complete(); + } + + [Fact] + public async Task MultipleGetReturn_SameTransaction_Async_ReusesConnection() + { + // Arrange + using var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled); + var transaction = Transaction.Current; + Assert.NotNull(transaction); + + // Act - get and return multiple times within same transaction + for (int i = 0; i < 10; i++) + { + var owner = new SqlConnection(); + var conn = await GetConnectionAsync(owner, transaction: transaction); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + } + + // Assert - only one connection should be in the transacted pool + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections[transaction]); + + scope.Complete(); + } + + [Fact] + public void AlternatingCommitAndRollback_MaintainsConsistentState() + { + // Act - alternate between commit and rollback + for (int i = 0; i < 20; i++) + { + using var scope = new TransactionScope(); + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + + if (i % 2 == 0) + { + scope.Complete(); + } + // else: rollback (no Complete) + } + + // Assert + AssertPoolMetrics(); + } + + #endregion + + #region Nested Transaction Tests + + [Fact] + public void NestedTransaction_Required_SharesSameTransactedEntry() + { + // Arrange + using var outerScope = new TransactionScope(); + var outerTxn = Transaction.Current; + Assert.NotNull(outerTxn); + + var owner1 = new SqlConnection(); + var conn1 = GetConnection(owner1); + Assert.NotNull(conn1); + ReturnConnection(conn1, owner1); + + // Act - nested scope with Required shares the same transaction + using (var innerScope = new TransactionScope(TransactionScopeOption.Required)) + { + Assert.Same(outerTxn, Transaction.Current); + + var owner2 = new SqlConnection(); + var conn2 = GetConnection(owner2); + Assert.NotNull(conn2); + Assert.Same(conn1, conn2); // Same transaction -> same connection from transacted pool + ReturnConnection(conn2, owner2); + + // Only one transaction tracked + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + + innerScope.Complete(); + } + + outerScope.Complete(); + } + + [Fact] + public void NestedTransaction_RequiresNew_CreatesSeparateTransactedEntry() + { + // Arrange + using var outerScope = new TransactionScope(); + var outerTxn = Transaction.Current; + Assert.NotNull(outerTxn); + + var owner1 = new SqlConnection(); + var conn1 = GetConnection(owner1); + Assert.NotNull(conn1); + ReturnConnection(conn1, owner1); + + // Act - nested scope with RequiresNew creates a new transaction + using (var innerScope = new TransactionScope(TransactionScopeOption.RequiresNew)) + { + var innerTxn = Transaction.Current; + Assert.NotNull(innerTxn); + Assert.NotEqual(outerTxn, innerTxn); + + var owner2 = new SqlConnection(); + var conn2 = GetConnection(owner2); + Assert.NotNull(conn2); + Assert.NotSame(conn1, conn2); // Different transaction -> different connection + ReturnConnection(conn2, owner2); + + // Two separate transactions tracked + Assert.Equal(2, _pool.TransactedConnectionPool.TransactedConnections.Count); + + innerScope.Complete(); + } + + outerScope.Complete(); + } + + [Fact] + public void NestedTransaction_RequiresNew_CompletesIndependently() + { + // Arrange & Act + using (var outerScope = new TransactionScope()) + { + var owner1 = new SqlConnection(); + var conn1 = GetConnection(owner1); + Assert.NotNull(conn1); + ReturnConnection(conn1, owner1); + + using (var innerScope = new TransactionScope(TransactionScopeOption.RequiresNew)) + { + var owner2 = new SqlConnection(); + var conn2 = GetConnection(owner2); + Assert.NotNull(conn2); + ReturnConnection(conn2, owner2); + innerScope.Complete(); + } + + // Inner transaction completed - its entry should be cleared + // Outer transaction entry should still exist + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + + outerScope.Complete(); + } + + // Both completed + AssertPoolMetrics(); + } + + [Fact] + public void DeeplyNestedTransactions_RequiresNew_AllTrackedSeparately() + { + // Arrange & Act + using var scope1 = new TransactionScope(); + var owner1 = new SqlConnection(); + var conn1 = GetConnection(owner1); + ReturnConnection(conn1, owner1); + + using var scope2 = new TransactionScope(TransactionScopeOption.RequiresNew); + var owner2 = new SqlConnection(); + var conn2 = GetConnection(owner2); + ReturnConnection(conn2, owner2); + + using var scope3 = new TransactionScope(TransactionScopeOption.RequiresNew); + var owner3 = new SqlConnection(); + var conn3 = GetConnection(owner3); + ReturnConnection(conn3, owner3); + + // Assert - three separate transactions tracked + Assert.Equal(3, _pool.TransactedConnectionPool.TransactedConnections.Count); + + scope3.Complete(); + scope2.Complete(); + scope1.Complete(); + } + + [Fact] + public void DeeplyNestedTransactions_Required_AllShareOneEntry() + { + // Arrange & Act + using var scope1 = new TransactionScope(); + var txn = Transaction.Current; + var owner1 = new SqlConnection(); + var conn1 = GetConnection(owner1); + ReturnConnection(conn1, owner1); + + using var scope2 = new TransactionScope(TransactionScopeOption.Required); + Assert.Same(txn, Transaction.Current); + var owner2 = new SqlConnection(); + var conn2 = GetConnection(owner2); + Assert.Same(conn1, conn2); + ReturnConnection(conn2, owner2); + + using var scope3 = new TransactionScope(TransactionScopeOption.Required); + Assert.Same(txn, Transaction.Current); + var owner3 = new SqlConnection(); + var conn3 = GetConnection(owner3); + Assert.Same(conn1, conn3); + ReturnConnection(conn3, owner3); + + // Assert - single transaction entry + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + + scope3.Complete(); + scope2.Complete(); + scope1.Complete(); + } + + #endregion + + #region Mixed Transacted and Non-Transacted Tests + + [Fact] + public void MixedWorkload_AlternatingTransactedAndNonTransacted() + { + // Act - alternate between transacted and non-transacted + for (int i = 0; i < 10; i++) + { + if (i % 2 == 0) + { + using var scope = new TransactionScope(); + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + scope.Complete(); + } + else + { + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + } + } + + // Assert + AssertPoolMetrics(); + } + + #endregion + + #region Shared Transaction Tests + + [Fact] + public void SharedTransaction_DependentScopes_UseTransactedPool() + { + // Arrange + using var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled); + var transaction = Transaction.Current; + Assert.NotNull(transaction); + + // Act - first connection + var owner1 = new SqlConnection(); + var conn1 = GetConnection(owner1); + Assert.NotNull(conn1); + ReturnConnection(conn1, owner1); + + // Use dependent scope on same transaction + using (var innerScope = new TransactionScope(transaction)) + { + Assert.Same(transaction, Transaction.Current); + var owner2 = new SqlConnection(); + var conn2 = GetConnection(owner2); + Assert.NotNull(conn2); + Assert.Same(conn1, conn2); // Same transaction -> same connection + ReturnConnection(conn2, owner2); + innerScope.Complete(); + } + + // Assert - still one transaction entry + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections[transaction]); + + scope.Complete(); + } + + #endregion + + #region Pool Saturation with Transactions Tests + + [Fact] + public void PoolSaturation_BlocksUntilConnectionAvailable() + { + // Arrange - small pool + _pool.Shutdown(); + _pool.Clear(); + _pool = CreatePool(maxPoolSize: 1); + + using var allAcquired = new ManualResetEventSlim(false); + using var releaseFirst = new ManualResetEventSlim(false); + + var saturatingTask = Task.Run(() => + { + using var scope = new TransactionScope(); + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + + allAcquired.Set(); // Signal that this connection is held + + Assert.True(releaseFirst.Wait(TimeSpan.FromSeconds(15)), + "Timed out waiting for releaseFirst signal."); + + ReturnConnection(conn, owner); + scope.Complete(); + }); + + Assert.True(allAcquired.Wait(TimeSpan.FromSeconds(10)), + "Timed out waiting for connection to be acquired."); + Assert.Equal(1, _pool.Count); + + using var acquired = new ManualResetEventSlim(false); + var waitingTask = Task.Run(() => + { + using var scope = new TransactionScope(); + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + acquired.Set(); + ReturnConnection(conn, owner); + scope.Complete(); + }); + + // Give the waiting task time to block — it should NOT complete yet + Assert.False(acquired.Wait(TimeSpan.FromMilliseconds(500)), + "Waiting task should not have acquired a connection while pool is saturated"); + + // Release one connection to unblock the waiting task + releaseFirst.Set(); + + // Now the waiting task should complete + Assert.True(waitingTask.Wait(TimeSpan.FromSeconds(15)), + "Waiting task should have completed after a connection was released"); + Assert.True(acquired.IsSet); + + // Cleanup remaining held connections + Task.WaitAll(saturatingTask); + } + + #endregion + + #region Controlled Concurrency Tests + + [Fact] + public void TwoThreads_SharedTransaction_AccessSameTransactedEntry() + { + // Arrange + // Use 3-phase synchronization so task1 gets AND returns before task2 requests. + // This ensures the connection is back in the transacted pool for task2 to reuse. + using var task1Returned = new ManualResetEventSlim(false); + using var task2Done = new ManualResetEventSlim(false); + DbConnectionInternal? connFromTask1 = null; + DbConnectionInternal? connFromTask2 = null; + + using var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled); + var transaction = Transaction.Current; + Assert.NotNull(transaction); + + // Act - two threads sharing the same transaction, sequenced so the + // transacted pool can vend the same connection to both. + var task1 = Task.Run(() => + { + using var innerScope = new TransactionScope(transaction); + var owner = new SqlConnection(); + connFromTask1 = GetConnection(owner); + Assert.NotNull(connFromTask1); + + // Return the connection so it's available in the transacted pool + ReturnConnection(connFromTask1, owner); + innerScope.Complete(); + + task1Returned.Set(); // Signal: connection is back in the transacted pool + }); + + var task2 = Task.Run(() => + { + // Wait until task1 has returned the connection to the transacted pool + Assert.True(task1Returned.Wait(TimeSpan.FromSeconds(10)), + "Timed out waiting for task1 to return its connection."); + + using var innerScope = new TransactionScope(transaction); + var owner = new SqlConnection(); + connFromTask2 = GetConnection(owner); + Assert.NotNull(connFromTask2); + ReturnConnection(connFromTask2, owner); + innerScope.Complete(); + }); + + Task.WaitAll(task1, task2); + + // Both tasks should have received the same connection via the transacted pool + Assert.Same(connFromTask1, connFromTask2); + scope.Complete(); + } + + [Fact] + public async Task TwoThreads_SeparateTransactions_Async_IsolatedTransactedEntries() + { + // Arrange + using var barrier = new SemaphoreSlim(0, 2); + + // Act + var task1 = Task.Run(async () => + { + using var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled); + var transaction = Transaction.Current; + var owner = new SqlConnection(); + var conn = await GetConnectionAsync(owner, transaction: transaction); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + + barrier.Release(); // Signal ready + await barrier.WaitAsync(); // Wait for other task + + scope.Complete(); + }); + + var task2 = Task.Run(async () => + { + using var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled); + var transaction = Transaction.Current; + var owner = new SqlConnection(); + var conn = await GetConnectionAsync(owner, transaction: transaction); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + + barrier.Release(); // Signal ready + await barrier.WaitAsync(); // Wait for other task + + scope.Complete(); + }); + + await Task.WhenAll(task1, task2); + + // Assert + AssertPoolMetrics(); + } + + #endregion + + #region Pool Shutdown with Transactions Tests + + [Fact] + public void PoolShutdown_AfterTransactionComplete_NoLeaks() + { + // Arrange + using (var scope = new TransactionScope()) + { + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + scope.Complete(); + } + + // Act + _pool.Shutdown(); + + // Assert + AssertPoolMetrics(); + } + + [Fact] + public void PoolShutdown_WhileConnectionHeld_NoException() + { + // Arrange + using var scope = new TransactionScope(); + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + + // Act - shutdown while connection is held (not yet returned) + _pool.Shutdown(); + + // Return after shutdown — the pool deactivates and disposes the connection + // rather than returning it to the pool. Verify this doesn't throw. + ReturnConnection(conn, owner); + + // Assert + // The connection should have been deactivated and disposed (not returned to the pool). + // After Dispose(), IsConnectionDoomed is set to true and Pool is set to null. + Assert.True(conn.IsConnectionDoomed, + "Connection should be doomed after returning to a shut-down pool."); + Assert.Null(conn.Pool); + } + + #endregion + + #region Transaction Complete Before Return Tests + + [Fact] + public void TransactionComplete_ThenReturn_ConnectionStillReturned() + { + // Arrange + var owner = new SqlConnection(); + DbConnectionInternal conn; + + using (var scope = new TransactionScope()) + { + conn = GetConnection(owner); + Assert.NotNull(conn); + scope.Complete(); + } + // Transaction is fully disposed here + + // Act - return connection after transaction ended + ReturnConnection(conn, owner); + + // Assert - no leak, pool metrics consistent + AssertPoolMetrics(); + Assert.True(_pool.Count > 0, "Pool should still have the connection"); + } + + #endregion + + #region Sequential Transaction Isolation Tests + + [Fact] + public void SequentialTransactions_EachGetsOwnTransactedEntry() + { + // Act - create multiple sequential transactions + for (int i = 0; i < 5; i++) + { + using var scope = new TransactionScope(); + var transaction = Transaction.Current; + Assert.NotNull(transaction); + + var owner = new SqlConnection(); + var conn = GetConnection(owner); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + + // Only the current transaction should be tracked + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + Assert.True(_pool.TransactedConnectionPool.TransactedConnections.ContainsKey(transaction)); + + scope.Complete(); + } + + // Assert - after all are done, pool should be clean + AssertPoolMetrics(); + } + + [Fact] + public async Task SequentialTransactions_Async_EachGetsOwnTransactedEntry() + { + // Act - create multiple sequential transactions + for (int i = 0; i < 5; i++) + { + using var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled); + var transaction = Transaction.Current; + Assert.NotNull(transaction); + + var owner = new SqlConnection(); + var conn = await GetConnectionAsync(owner, transaction: transaction); + Assert.NotNull(conn); + ReturnConnection(conn, owner); + + Assert.Single(_pool.TransactedConnectionPool.TransactedConnections); + Assert.True(_pool.TransactedConnectionPool.TransactedConnections.ContainsKey(transaction)); + + scope.Complete(); + } + + // Assert + AssertPoolMetrics(); + } + + [Fact] + public void SequentialTransactions_CanReuseConnections() + { + // Act + DbConnectionInternal conn1; + DbConnectionInternal conn2; + Transaction? txn1; + Transaction? txn2; + + using (var scope1 = new TransactionScope()) + { + txn1 = Transaction.Current; + var owner1 = new SqlConnection(); + conn1 = GetConnection(owner1); + Assert.NotNull(conn1); + ReturnConnection(conn1, owner1); + scope1.Complete(); + } + + using (var scope2 = new TransactionScope()) + { + txn2 = Transaction.Current; + var owner2 = new SqlConnection(); + conn2 = GetConnection(owner2); + Assert.NotNull(conn2); + ReturnConnection(conn2, owner2); + scope2.Complete(); + } + + // Assert + // The connection was returned to the general pool and picked up by the second transaction + Assert.NotSame(txn1, txn2); + Assert.Same(conn1, conn2); + AssertPoolMetrics(); + } + + #endregion + + #region Mock Classes + + internal class MockSqlConnectionFactory : SqlConnectionFactory + { + protected override DbConnectionInternal CreateConnection( + DbConnectionOptions options, + DbConnectionPoolKey poolKey, + DbConnectionPoolGroupProviderInfo poolGroupProviderInfo, + IDbConnectionPool pool, + DbConnection owningConnection, + DbConnectionOptions userOptions) + { + return new MockDbConnectionInternal(); + } + } + + internal class MockDbConnectionInternal : DbConnectionInternal + { + private static int s_nextId = 1; + public int MockId { get; } = Interlocked.Increment(ref s_nextId); + + public override string ServerVersion => "Mock"; + + public override DbTransaction BeginTransaction(System.Data.IsolationLevel il) + { + throw new NotImplementedException(); + } + + public override void EnlistTransaction(Transaction? transaction) + { + if (transaction != null) + { + EnlistedTransaction = transaction; + } + } + + protected override void Activate(Transaction? transaction) + { + EnlistedTransaction = transaction; + } + + protected override void Deactivate() + { + } + + public override string ToString() => $"MockConnection_{MockId}"; + + internal override void ResetConnection() + { + } + } + + #endregion +}