using lock statement with command object in multithreaded app

[timb]

Hi,
i have a function which i use to insert records in my SQL2K database.
The function is used to add records to a table which is used as a log or
audit trail of what operations occured in the app at what time.

My app is multithreaded so i have used a lock on the command object
"SQLcmdFT" is this correct practice?(note in the example below i have
laready instantiated the object and added the parameters.)

If i step through the code i have no problems but if i run it the connection
object appears to close without me calling a close?

private static string SetFileTransferStatus(string Filename,string
Direction,I8FileTransferStatus Status,I8FileTransferStatus NewStatus)

{

try

{

lock(SQLcmdFT)

{

if (Filename == null) Filename = "";

if (Direction == null) Direction = "";

SQLcmdFT.Parameters["@FileName"].Value = Filename;

SQLcmdFT.Parameters["@Status"].Value = Status;

SQLcmdFT.Parameters["@Direction"].Value = Direction;

SQLcmdFT.Parameters["@NewStatus"].Value = NewStatus;

SQLcmdFT.ExecuteNonQuery();

return SQLcmdFT.Parameters["@Filename"].Value.ToString();

}

}

catch(Exception e)

{

LogEvent(0,false,e.Message);

return null;

}

}

[Nicholas Paldino [.NET/C# MVP]]

timb,

If you pass a connection to a command, and the connection is closed,
then any call on the command will cause the command to open the connection,
use it, and then close it. Basically, the command will leave the connection
in the state it was in before you made the call.

Also, I highly doubt that you have to place a lock on this code. It
doesn't seem to be accessing any class level variables. I might be
mistaken, but I ^think^ SQL2K can handle concurrent requests correctly (I'm
being sarcastic!).

Also, if you are concerned about the LogEvent method being thread-safe,
then you should place locks in there to lock access to it.

Hope this helps.

--
- Nicholas Paldino [.NET/C# MVP]
- nicholas...@exisconsulting.com

"timb" <ti...@smurf.com> wrote in message
news:%23zeaHoG...@TK2MSFTNGP12.phx.gbl...

[timb]

Thanks Nicolas,
this particular command object is attached to an open connection
object.
I have resolved the problem with the connection closing and other
such bizarre errors ("general network error") by creating a seperate
connection object for this command object. Therefore all other commands are
using another connection object.(not an ideal but a workaround)

Also without placing the lock on this method or similar command calling
functions is it not possible that if two threads are entering the same
function that the second thread could change the parameters after the first
thread has changed them but before the command is executed?

Also i was (before the bizarre connection closing problems) reusing the same
command object for calling several different stored procedures.The lock
statement in this context should have prevented threads from clearing the
parameters collection of the command object when a thread was setting the
parameters of the command and executing it. Is reuse of the same command
object more efficient in terms of memory? i.e. if i have to call 10
different update stored procedures is it more memory efficient to use as few
command objects as possible?

thanks greatly in advance

Tim B

"Nicholas Paldino [.NET/C# MVP]" <nicholas...@exisconsulting.com> wrote
in message news:e12RLpJF...@TK2MSFTNGP10.phx.gbl...

[Nicholas Paldino [.NET/C# MVP]]

Tim,

You shouldn't keep a connection open and lying around. The
SqlConnection class uses a connection pool in the background, and keeping
your connection open is actually going to degrade your performance. You
will want to create a new connection, open it, use it, and then close it for
every operation that you do (or group of operations). You shouldn't be
holding the connection open across operations. You will also notice MUCH
better performance when you do this.

The same thing holds with the command object. Don't use the same one.
Use new ones. If you want to use a command object over and over again, you
can, but use it for the same parameterized command. Don't keep changing the
command string. In this case, you might want to place a lock on the command
for multi-thread access. However, this can be a performance issue as well.
If anything, have a method that will construct a new command instance with
the command you want to execute, as well as set up the parameters.

--
- Nicholas Paldino [.NET/C# MVP]
- nicholas...@exisconsulting.com

"timb" <ti...@smurf.com> wrote in message

news:uppbfzJF...@TK2MSFTNGP12.phx.gbl...

[timb]

Hi Nicolas,
ok i have changed my method as follows. This method is called within
a loop once every second. Do i need to force a GC.Collect() every 1000
iterations or so to reduce memory overheads?

private static string SetFileTransferStatus(string Filename,string
Direction,I8FileTransferStatus Status,I8FileTransferStatus NewStatus)

{

try

{

//lock(SQLcmdFT)

//{

if (Filename == null) Filename = "";

if (Direction == null) Direction = "";

SQLcmdFT = new System.Data.SqlClient.SqlCommand();

SQLcmdFT.CommandText = "[EPOSSYNC_FileTransfers_GetFile]";

SQLcmdFT.CommandType = CommandType.StoredProcedure;

SQLcmdFT.Connection = SQLcnSync;

SQLcmdFT.Parameters.Clear();

SQLcmdFT.Parameters.Add("@FullFileName",SqlDbType.VarChar,255);

SQLcmdFT.Parameters["@FullFileName"].Direction =
System.Data.ParameterDirection.InputOutput;

SQLcmdFT.Parameters.Add("@Status",SqlDbType.TinyInt);

SQLcmdFT.Parameters["@Status"].Direction =
System.Data.ParameterDirection.InputOutput;

SQLcmdFT.Parameters.Add("@Direction",SqlDbType.Char,1);

SQLcmdFT.Parameters.Add("@NewStatus",SqlDbType.TinyInt);

SQLcmdFT.Parameters["@FullFileName"].Value = Filename;

SQLcmdFT.Parameters["@Status"].Value = Status;

SQLcmdFT.Parameters["@Direction"].Value = Direction;

SQLcmdFT.Parameters["@NewStatus"].Value = NewStatus;

SQLcmdFT.ExecuteNonQuery();

Filename = SQLcmdFT.Parameters["@FullFilename"].Value.ToString();

SQLcmdFT.Dispose();

return Filename;

//}

}

catch(Exception e)

{

LogEvent(0,false,e.Message);

return null;

}

}

thanks in advance

Tim B

"Nicholas Paldino [.NET/C# MVP]" <nicholas...@exisconsulting.com> wrote

in message news:eYSDZ%23JFDH...@TK2MSFTNGP10.phx.gbl...

[Daniel Jin]

you are still sharing the command object right? I assume
the SQLcmdFT is a static variable belonging to your
class. the problem of doing it this way, is you are
forcing everything to go through one command and
connection. Imagine you have a few hundred threads trying
to call this method concurrently, they have to wait on
each other to finish. That one connection becomes the
bottleneck when there could have been 30 or 40 unused
connections available.

and I don't think you need to call GC.Collect.

>.
>