search

dataplat / dbatools

🚀 SQL Server automation and instance migrations have never been safer, faster or freer
https://dbatools.io
MIT License
2.39k stars 788 forks source link

SqlParameters are not cleared after Invoke-DbaAsync completes, preventing reuse of SqlParameter objects #9217

Closed chadbaldwin closed 5 months ago

chadbaldwin commented 6 months ago

High level summary of the issue...

If you try to create a SqlParameter, save it to a variable and then use it more than once, you receive an exception.

But it seems if a simple call to $cmd.Parameters.Clear() is made in Invoke-DbaAsync, that seems to resolve the issue.

I guess when you add a SqlParameter to a SqlCommand, it gets claimed in some way. So if you try to use it somewhere else, you get an error saying it's already contained by another SqlParameterCollection.

Verified issue does not already exist?

I have searched and found no existing issue

What error did you receive?

Exception             : 
    Type           : System.Exception
    Message        : The SqlParameter is already contained by another SqlParameterCollection.
    InnerException : 
        Type           : System.Management.Automation.MethodInvocationException
        ErrorRecord    : 
            Exception             : 
                Type    : System.Management.Automation.ParentContainsErrorRecordException
                Message : Exception calling "Add" with "1" argument(s): "The SqlParameter is already contained by another SqlParameterCollection."
                HResult : -2146233087
            CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException
            FullyQualifiedErrorId : ArgumentException
            InvocationInfo        : 
                ScriptLineNumber : 93585
                OffsetInLine     : 25
                HistoryId        : -1
                Line             : $null = $cmd.Parameters.Add($sqlparam)

                PositionMessage  : At line:93585 char:25
                                   +                         $null = $cmd.Parameters.Add($sqlparam)
                                   +                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                CommandOrigin    : Internal
            ScriptStackTrace      : at Invoke-DbaAsync<Process>, <No file>: line 93585
                                    at Invoke-DbaQuery<Process>, <No file>: line 52421
                                    at <ScriptBlock>, <No file>: line 1
        TargetSite     : 
            Name          : CheckActionPreference
            DeclaringType : System.Management.Automation.ExceptionHandlingOps, System.Management.Automation, Version=7.3.10.500, Culture=neutral, PublicKeyToken=31bf3856ad364e35
            MemberType    : Method
            Module        : System.Management.Automation.dll
        Message        : Exception calling "Add" with "1" argument(s): "The SqlParameter is already contained by another SqlParameterCollection."
        Data           : System.Collections.ListDictionaryInternal
        InnerException : 
            Type       : System.ArgumentException
            Message    : The SqlParameter is already contained by another SqlParameterCollection.
            TargetSite : 
                Name          : Validate
                DeclaringType : Microsoft.Data.SqlClient.SqlParameterCollection
                MemberType    : Method
                Module        : Microsoft.Data.SqlClient.dll
            Source     : Microsoft.Data.SqlClient
            HResult    : -2147024809
            StackTrace : 
   at Microsoft.Data.SqlClient.SqlParameterCollection.Validate(Int32 index, Object value)
   at Microsoft.Data.SqlClient.SqlParameterCollection.Add(Object value)
   at CallSite.Target(Closure, CallSite, Object, Object)
        Source         : System.Management.Automation
        HResult        : -2146233087
        StackTrace     : 
   at System.Management.Automation.ExceptionHandlingOps.CheckActionPreference(FunctionContext funcContext, Exception exception)
   at System.Management.Automation.Interpreter.ActionCallInstruction`2.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.EnterTryCatchFinallyInstruction.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.EnterTryCatchFinallyInstruction.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.Interpreter.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.LightLambda.RunVoid1[T0](T0 arg0)
   at System.Management.Automation.PSScriptCmdlet.RunClause(Action`1 clause, Object dollarUnderbar, Object inputToProcess)
   at System.Management.Automation.PSScriptCmdlet.DoProcessRecord()
   at System.Management.Automation.CommandProcessor.ProcessRecord()
    HResult        : -2146233088
TargetObject          : {{INSTANCENAME}}
CategoryInfo          : NotSpecified: ({{INSTANCENAME}}:PSObject) [Write-Error], Exception
FullyQualifiedErrorId : dbatools_Invoke-DbaQuery,Stop-Function
ErrorDetails          : The SqlParameter is already contained by another SqlParameterCollection.
InvocationInfo        : 
    MyCommand        : Stop-Function
    ScriptLineNumber : 52424
    OffsetInLine     : 17
    HistoryId        : 2
    Line             : Stop-Function -Message "[$instance] Failed during execution" -ErrorRecord $_ -Target $instance -Continue

    PositionMessage  : At line:52424 char:17
                       + …             Stop-Function -Message "[$instance] Failed during executi …
                       +               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    InvocationName   : Stop-Function
    CommandOrigin    : Internal
ScriptStackTrace      : at Stop-Function, <No file>: line 97916
                        at Invoke-DbaQuery<Process>, <No file>: line 52424
                        at <ScriptBlock>, <No file>: line 1
PipelineIterationInfo :

Steps to Reproduce

$conn = Connect-DbaInstance -SqlInstance MyServer -Database MyDB

$MyParam = New-DbaSqlParameter -ParameterName 'MyParam' -SqlDbType Int -Value 10

# Works fine as expected
Invoke-DbaQuery $conn -CommandType StoredProcedure -Query 'dbo.usp_TestParams' -SqlParameter $MyParam

# Will fail and throw exception
Invoke-DbaQuery $conn -CommandType StoredProcedure -Query 'dbo.usp_TestParams' -SqlParameter $MyParam

Please confirm that you are running the most recent version of dbatools

2.1.6

Other details or mentions

I'm pretty sure this is the change that needs to be made:

This...

# Clear parameters to allow for their reuse in subsequent calls
$cmd.Parameters.Clear()

Needs to go around here...(between these two lines) https://github.com/dataplat/dbatools/blob/56d716a32a0007d3963e1414ce8a27ff656b4ac3/private/functions/Invoke-DbaAsync.ps1#L294-L295

I made this change locally and it fixed the issue, but I don't know what it might blow up, so I decided to submit this as an issue rather than a PR. I don't know how to run all the pester tests and stuff.

What PowerShell host was used when producing this error

PowerShell Core (pwsh.exe)

PowerShell Host Version

Name                           Value
----                           -----
PSVersion                      7.3.10
PSEdition                      Core
GitCommitId                    7.3.10
OS                             Microsoft Windows 10.0.19045
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

SQL Server Edition and Build number

Microsoft SQL Server 2017 (RTM-CU31-GDR) (KB5029376) - 14.0.3465.1 (X64) 
    Jul 30 2023 15:31:58 
    Copyright (C) 2017 Microsoft Corporation
    Enterprise Edition: Core-based Licensing (64-bit) on Windows Server 2016 Standard 10.0 <X64> (Build 14393: )

.NET Framework Version

.NET 7.0.14
niphlod commented 6 months ago

not a bug per se because it's this way also on .NET but a welcome feature request. I'll look into it into the weekend

edit: thanks @chadbaldwin for the report

chadbaldwin commented 4 months ago

@niphlod Just an FYI, might not be worth fixing, but I at least wanted to make some note so y'all are aware of it.

I updated my scripts to utilize this change, however, I found that if there is an exception thrown prior to the .Clear() being run, then you can't re-use the parameter set. So this might mess some people/scripts up if they are looping and one iteration of that loop throws an exception, it's caught but is then configured to keep going.

In my case, I just need to fix the exception and I won't run into this issue anymore, so I can work around it.

PowerShell 7.3 added a clean block to functions, which could be used to resolve this by putting $cmd.Parameters.Clear() in the clean block. BUT, I'm going to assume that won't work since it kills all backwards compatibility anything lower than Pwsh 7.3.

Obviously another option would be to put the whole thing in a try/finally block.