Command executenonquery ошибка

При создании SqlConnection необходимо явно указывать поле Connection, иначе оно автоматически инициализируется в null.

Честно говоря, я не знаю, почему API позволяет конструировать невалидные объекты, но думаю, что это не design flaw, а все-таки служит какой-то цели.

В вашем случае нужно использовать:

var command = new SqlCommand(Query, Connection);

Кстати, да. SqlConnection — это IDisposable. Соответственно, если вы храните его как поле в вашем классе, то не обходимо делать класс IDisposable и предоставлять методы для освобождения ресурсов (иначе в ближайшем будущем вы можете столкнуться с проблемами).

Более простой вариант — использовать connection в локальном скоупе через using.

Ваш текущий вариант с using неправильный, т.к произойдет следующее:

• В методе Connect будет создан объект Connection.
• После первого вызова метода SQLWithoutFetch (на выходе из using блока) будет неявно вызван Connection.Dispose().
• При любой следующей попытке вызвать SQLWithoutFetch вы (в лучшем случае) получите ObjectDisposedException, поскольку объект был освобожден в предыдущем вызове.

Как решать это я написал выше.

Еще одно замечание:

catch (SqlException)
{
    throw new Exception("Bad SQL query");
}

Заменять конкретный SqlException с параметрами ошибки на общий Exception недопустимо. try-catch здесь вообще не нужен, т.к вы не можете ничего сделать внутри этого метода, чтобы исправить ошибочную ситуацию.

Обработка ошибок должна быть уровнем выше, следовательно, автоматически проброшенный SqlException — это хорошо.

А, и еще:

if (Connection.State == ConnectionState.Open) ...

I have a very strange problem that only occurs when the code in question is in a high load situations. My ASP.NET web client C# code calls a T-SQL stored procedure that has an OUTPUT parameter.

Under high loads, the data being returned sometimes does not make it back to the calling C# code. I have extracted all the relevant code into the example below;

Stored procedure:

CREATE PROCEDURE GetLoginName
    @LoginId BIGINT,
    @LoginName VARCHAR(50) OUTPUT
AS
   SET NOCOUNT ON

   SELECT @LoginName = LoginName
   FROM Logins
   WHERE Id = @LoginId

   SET NOCOUNT OFF
GO

Database base class:

public class DatabaseContextBase : IDisposable
{
    private SqlConnection _connection;
    private string _connectionString;
    private SqlInt32 _returnValue;
    private int _commandTimeout;
    private static int _maxDatabaseExecuteAttempts = 3;   

    private static int s_commandTimeout = 30;

    protected DBContextBase()
    {
        // try and get the connection string off the identity first...
        string connectionString = GetConnectionStringFromIdentity();

        if(connectionString != null)
        {
            ConstructionHelper(connectionString, s_commandTimeout);
        }
        else
        {
            // use the initialised static connection string, and call the other overload
            // of the constructor
            ConstructionHelper(s_connectionString, s_commandTimeout);
        }   
    }

    private void ConstructionHelper( string connectionString, int commandTimeout ) 
    {
        // store the connection string in a member var.
        _connectionString = connectionString;

        // store the timeout in a member var.
        _commandTimeout = commandTimeout;
    }

    public static string GetConnectionStringFromIdentity()
    {
        IIdentity identity = Thread.CurrentPrincipal.Identity as wxyz.Security.wxyzIdentityBase;
        string connectionString = null;

        if(identity != null)
        {
            connectionString = ((wxyz.Security.wxyzIdentityBase) identity ).ConnectionString;
        }

        return connectionString;
    }

    public void Dispose()
    {           
        if (_connection.State != ConnectionState.Closed)
        {
            _connection.Close();
        }

        _connection.Dispose();
        _connection = null;
    }

    protected void ExecuteNonQuery(SqlCommand command)
    {
        SqlConnection con = this.Connection;

        lock (con)
        {
            if (con.State != ConnectionState.Open)
            {
                con.Open();
            }

            // don't need a try catch as this is only ever called from another method in this 
            // class which will wrap it.
            command.Connection = con;
            command.Transaction = _transaction;
            command.CommandTimeout = _commandTimeout;

            for (int currentAttempt = 1; currentAttempt == _maxDatabaseExecuteAttempts; currentAttempt++)
            {
                try
                {
                    // do it
                    command.ExecuteNonQuery();

                    // done, exit loop
                    break;
                }
                catch (SqlException sex)
                {
                    HandleDatabaseExceptions(currentAttempt, sex, command.CommandText);
                }
            }
        }   
    }

    protected void HandleDatabaseExceptions(int currentAttempt, SqlException sqlException, string sqlCommandName)
    {
        if (DataExceptionUtilities.IsDeadlockError(sqlException))
        {
            if (!this.IsInTransaction)
            {
                // Not in a transaction and a deadlock detected.
                // If we have not exceeded our maximum number of attemps, then try to execute the SQL query again.
                string deadlockMessage = string.Format("Deadlock occured in attempt {0} for '{1}':", currentAttempt, sqlCommandName);
                Logging.Write(new ErrorLogEntry(deadlockMessage, sqlException));

                if (currentAttempt == DBContextBase.MaxDatabaseExecuteAttempts)
                {
                    // This was the last attempt so throw the exception
                    throw sqlException;
                }

                // Wait for a short time before trying again
                WaitShortAmountOfTime();
            }
            else
            {
                // We're in a transaction, then the calling code needs to handle the deadlock
                string message = string.Format("Deadlock occured in transaction for '{0}':", sqlCommandName);

                throw new DataDeadlockException(message, sqlException);
            }
        }
        else if (this.IsInTransaction && DataExceptionUtilities.IsTimeoutError(sqlException))
        {
            // We're in a transaction and the calling code needs to handle the timeout 
            string message = string.Format("Timeout occured in transaction for '{0}':", sqlCommandName);

            // Raise a Deadlock exception and the calling code will rollback the transaction
            throw new DataDeadlockException(message, sqlException);
        }
        else
        {
            // Something else has gone wrong
            throw sqlException;
        }   
    }

    /// <summary>
    /// get the SqlConnection object owned by this database (already connected to db) 
    /// </summary>
    public SqlConnection Connection
    {
        get {
            // check whether we've got a connection string (from either identity or static initialise)
            if ( _connectionString == null )
            {
                throw new ArgumentNullException( "connectionString", "Connection string not set" );
            }

            if ( _connection != null )
            {
                return _connection;
            }
            else
            {
                _connection = new SqlConnection( _connectionString );
                return _connection;
            }
        }   
    }

    /// <summary>
    /// Return value from executed stored procedure
    /// </summary>
    public SqlInt32 ReturnValue
    {
        get { return _returnValue; }
        set { _returnValue = value; }
    }
}

Database access class:

public class AuthenticationDBCommands
{
    public static SqlCommand GetLoginName()
    {
        System.Data.SqlClient.SqlCommand cmd = new System.Data.SqlClient.SqlCommand("GetLoginName");
        cmd.CommandType = CommandType.StoredProcedure;

        cmd.Parameters.Add(new SqlParameter("@RETURN_VALUE", System.Data.SqlDbType.Int, 0, System.Data.ParameterDirection.ReturnValue, false, 0, 0, "RETURN_VALUE", System.Data.DataRowVersion.Current, SqlInt32.Null));

        cmd.Parameters.Add(new SqlParameter("@LoginId", SqlDbType.BigInt, 8, ParameterDirection.Input, false, 0, 0, "LoginId", DataRowVersion.Current, SqlInt64.Null));

        cmd.Parameters.Add(new SqlParameter("@LoginName", SqlDbType.VarChar, 50, ParameterDirection.InputOutput,false, 0, 0, "LoginName", DataRowVersion.Current, SqlString.Null));

        return cmd;
    }
}

public class AuthenticationDBContext : DatabaseContextBase
{
    public AuthenticationDBContext() : base()
    {
    } 

    public void GetLoginName(SqlInt64 LoginId, ref SqlString LoginName)
    {
        SqlCommand cmd = AuthenticationDBCommands.GetLoginName();
        cmd.Parameters[1].Value = LoginId;
        cmd.Parameters[2].Value = LoginName;

        base.ExecuteNonQuery(cmd);
        base.ReturnValue = (SqlInt32) cmd.Parameters[0].Value; 
        LoginName = (SqlString)(cmd.Parameters[2].Value); 
    }
}

So when it’s used inside the ASP.NET web client it look like this:

protected string GetLoginName(long loginId)
{
    SqlString loginName = SqlString.Null;

    using (AuthenticationDBContext dbc = new AuthenticationDBContext())
    {
      dbc.GetLoginName(loginId, ref loginName);
    }

    return loginName.Value;
}

As you can see this is fairly standard stuff. But when the AuthenticationDBContext.GetLoginName() method is called by many different users in quick succession the loginName object is sometimes null.

When the SqlCommand.ExecuteNonQuery() fails to return any data it does not throw an exception.

I have tested the SQL and it always finds a value (I’ve inserted @LoginName into a table and it’s never null). So the problem is happening after or in SqlCommand.ExecuteNonQuery();

As I said, this is an example of what it happening. In reality, data is not being returned for lots of different stored procedures.

It’s also worth stating that other web clients that share the app pool in IIS are not affected when the web client in question is under a heavy load.

I’m using .NET 4.5 and my database is on SQL Server 2008.

Has anyone seen anything like this before?
Can anyone recommend any changes?

Thanks in advance,

Matt

UPDATE. Thanks for all your comments. I have made the following change to the DatabaseContextBase class.

                private void ExecuteNonQueryImpl(SqlCommand command)
            {
                object _lockObject = new object();

                lock (_lockObject)
                {
                    SqlConnection con = this.GetConnection();
                    if (con.State != ConnectionState.Open)
                    {
                        con.Open();
                    }

                    // don't need a try catch as this is only ever called from another method in this 
                    // class which will wrap it.
                    command.Connection = con;
                    command.Transaction = _transaction;
                    command.CommandTimeout = _commandTimeout;

                    for (int currentAttempt = 1; currentAttempt <= _maxDatabaseExecuteAttempts; currentAttempt++)
                    {
                        try
                        {
                            // do it
                            command.ExecuteNonQuery();

                            // done, exit loop
                            break;
                        }
                        catch (SqlException sex)
                        {
                            HandleDatabaseExceptions(currentAttempt, sex, command.CommandText);
                        }
                    }

                    if (!this.IsInTransaction)
                    {
                        con.Close();
                    }
                }
            }

            public SqlConnection GetConnection()
            {
                if (this.IsInTransaction)
                {
                    return this.Connection;
                }
                else
                {
                    // check whether we've got a connection string (from either identity or static initialise)
                    if ( _connectionString == null )
                    {
                        string exceptionMessage = Language.Translate("DbContextNotInitialized");

                        throw new ArgumentNullException( "connectionString", exceptionMessage );
                    }

                    return new SqlConnection(_connectionString);
                }
            }

However, in a load test the data still sometimes comes back as null. The web client is not working in a transaction so a new SqlConnection object is created, opened and closed every time a call is made. (there are other areas of code which share the DatabaseContextBase class that do work in a transaction so the original connection property is needed)

I would like to mention that again that I’m confident that the store procedure is working correctly as I have inserted the @LoginName value into a table and it’s never null.

Thanks,
Matt

Code Snippet

public override void SetPropertyValues(SettingsContext context, SettingsPropertyValueCollection collection) {

string username = (string)context[«UserName»];

bool userIsAuthenticated = (bool)context[«IsAuthenticated»];

if (username == null || username.Length < 1 || collection.Count < 1)

return;

SqlConnection conn = null;

SqlDataReader reader = null;

SqlCommand cmd = null;

try {

bool anyItemsToSave = false;

// First make sure we have at least one item to save

foreach (SettingsPropertyValue pp in collection) {

if (pp.IsDirty) {

if (!userIsAuthenticated) {

bool allowAnonymous = (bool)pp.Property.Attributes[«AllowAnonymous»];

if (!allowAnonymous)

continue;

}

anyItemsToSave = true;

break;

}

}

if (!anyItemsToSave)

return;

conn = new SqlConnection(_sqlConnectionString);

conn.Open();

List<ProfileColumnData> columnData = new List<ProfileColumnData>(collection.Count);

foreach (SettingsPropertyValue pp in collection) {

if (!userIsAuthenticated) {

bool allowAnonymous = (bool)pp.Property.Attributes[«AllowAnonymous»];

if (!allowAnonymous)

continue;

}

//Normal logic for original SQL provider

//if (!pp.IsDirty && pp.UsingDefaultValue) // Not fetched from DB and not written to

//Can eliminate unnecessary updates since we are using a table though

if (!pp.IsDirty)

continue;

string persistenceData = pp.Property.Attributes[«CustomProviderData»] as string;

// If we can’t find the table/column info we will ignore this data

if (String.IsNullOrEmpty(persistenceData)) {

// REVIEW: Perhaps we should throw instead?

continue;

}

string[] chunk = persistenceData.Split(new char[] { ‘;’ });

if (chunk.Length != 2) {

// REVIEW: Perhaps we should throw instead?

continue;

}

string columnName = chunk[0];

// REVIEW: Should we ignore case?

SqlDbType datatype = (SqlDbType)Enum.Parse(typeof(SqlDbType), chunk[1], true);

object value = null;

// REVIEW: Is this handling null case correctly?

if (pp.Deserialized && pp.PropertyValue == null) { // is value null?

value = DBNull.Value;

}

else {

value = pp.PropertyValue;

}

// REVIEW: Might be able to ditch datatype

columnData.Add(new ProfileColumnData(columnName, pp, value, datatype));

}

// Figure out userid, if we don’t find a userid, go ahead and create a user in the aspnetUsers table

Guid userId = Guid.Empty;

cmd = new SqlCommand(«SELECT u.UserId FROM vw_aspnet_Users u WHERE u.ApplicationId = ‘» + AppId + «‘ AND u.UserName = LOWER(@Username)», conn);

cmd.CommandType = CommandType.Text;

cmd.Parameters.AddWithValue(«@Username»,username);

try {

reader = cmd.ExecuteReader();

if (reader.Read()) {

userId = reader.GetGuid(0);

}

else {

reader.Close();

cmd.Dispose();

reader = null;

cmd = new SqlCommand(«dbo.aspnet_Users_CreateUser», conn);

cmd.CommandType = CommandType.StoredProcedure;

cmd.Parameters.AddWithValue(«@ApplicationId», AppId);

cmd.Parameters.AddWithValue(«@UserName», username);

cmd.Parameters.AddWithValue(«@IsUserAnonymous», !userIsAuthenticated);

cmd.Parameters.AddWithValue(«@LastActivityDate», DateTime.UtcNow);

cmd.Parameters.Add(CreateOutputParam(«@UserId», SqlDbType.UniqueIdentifier, 16));

cmd.ExecuteNonQuery();

userId = (Guid)cmd.Parameters[«@userid»].Value;

}

}

finally {

if (reader != null) {

reader.Close();

reader = null;

}

cmd.Dispose();

}

// Figure out if the row already exists in the table and use appropriate SELECT/UPDATE

cmd = new SqlCommand(String.Empty, conn);

StringBuilder sqlCommand = new StringBuilder(«IF EXISTS (SELECT 1 FROM «).Append(_table);

sqlCommand.Append(» WHERE UserId = @UserId) «);

cmd.Parameters.AddWithValue(«@UserId», userId);

// Build up strings used in the query

StringBuilder columnStr = new StringBuilder();

StringBuilder valueStr = new StringBuilder();

StringBuilder setStr = new StringBuilder();

int count = 0;

foreach (ProfileColumnData data in columnData) {

columnStr.Append(«, «);

valueStr.Append(«, «);

columnStr.Append(data.ColumnName);

string valueParam = «@Value» + count;

valueStr.Append(valueParam);

cmd.Parameters.AddWithValue(valueParam, data.Value);

// REVIEW: Can’t update Timestamps?

if (data.DataType != SqlDbType.Timestamp) {

if (count > 0) {

setStr.Append(«,»);

}

setStr.Append(data.ColumnName);

setStr.Append(«=»);

setStr.Append(valueParam);

}

++count;

}

columnStr.Append(«,LastUpdatedDate «);

valueStr.Append(«,@LastUpdatedDate»);

setStr.Append(«,LastUpdatedDate=@LastUpdatedDate»);

cmd.Parameters.AddWithValue(«@LastUpdatedDate», DateTime.UtcNow);

sqlCommand.Append(«BEGIN UPDATE «).Append(_table).Append(» SET «).Append(setStr.ToString());

sqlCommand.Append(» WHERE UserId = ‘»).Append(userId).Append(«‘»);

sqlCommand.Append(«END ELSE BEGIN INSERT «).Append(_table).Append(» (UserId»).Append(columnStr.ToString());

sqlCommand.Append(«) VALUES (‘»).Append(userId).Append(«‘»).Append(valueStr.ToString()).Append(«) END»);

cmd.CommandText = sqlCommand.ToString();

cmd.CommandType = CommandType.Text;

cmd.ExecuteNonQuery(); //THIS cmd.ExecuteNonQuery Produces the Error

// Need to close reader before we try to update

if (reader != null) {

reader.Close();

reader = null;

}

UpdateLastActivityDate(conn, userId);

}

finally {

if (reader != null)

reader.Close();

if (cmd != null)

cmd.Dispose();

if (conn != null)

conn.Close();

}

}

Вам нужно назначить соединение SqlCommand, вы можете использовать конструктор или свойство:

cmd.InsertCommand = new SqlCommand("INSERT INTO Application VALUES (@EventLog, @TimeGenerated, @EventType, @SourceName, @ComputerName, @InstanceId, @Message) ");
cmd.InsertCommand.Connection = connection1;

Я настоятельно рекомендую использовать using-statement для любого типа, реализующего IDisposable как SqlConnection, он также закроет соединение:

using(var connection1 = new SqlConnection(@"Data Source=.\sqlexpress;Initial Catalog=syslog2;Integrated Security=True"))
{
    SqlDataAdapter cmd = new SqlDataAdapter();  
    using(var insertCommand = new SqlCommand("INSERT INTO Application VALUES (@EventLog, @TimeGenerated, @EventType, @SourceName, @ComputerName, @InstanceId, @Message) "))
    {
        insertCommand.Connection = connection1;
        cmd.InsertCommand = insertCommand;
        //.....
        connection1.Open();
        // .... you don't need to close the connection explicitely
    }
}

Кроме того, вам не нужно создавать новое соединение и DataAdapter для каждой записи в foreach, даже если создание, открытие и закрытие соединения означает не, означает, что ADO.NET создаст, откроет и закроет соединение физическое, но просто заглянет в пул соединений для доступного подключения. Тем не менее это лишние накладные расходы.