Sql server deadlock retry

  • Thread starter Thread starter Raymon
  • Start date Start date
R

Raymon

I have this method in a data utility class used to retrieve data from
sql server.
It only checks for sever deadlock in which case it will retry the
operation 10 times before giving up.
Any other error is thrown back to the caller.

When I run code analysis on this it gives the warning datatable dt
is not disposed of in all exception paths.
How can I dispose of the datatable?
It is the very data that is being returned.

public static DataTable GetTable(SqlCommand cmd)
{
bool done = false;
int tries = 0;
DataTable dt = new DataTable();

while (!done)
{
try
{
using (SqlDataAdapter adp = new SqlDataAdapter(cmd))
{
adapter.Fill(dt);
}
done = true;
}
catch (SqlException e)
{
foreach (SqlError er in e.Errors)
{
if (er.Number == 1205 && tries < 10)
{
tries++;
System.Threading.Thread.Sleep(5);
break;
}
else
{
throw;
}
}
}
}
return dt;
}
 
I have this method in a data utility class used to retrieve data from
sql server.
It only checks for sever deadlock in which case it will retry the
operation 10 times before giving up.
Any other error is thrown back to the caller.

When I run code analysis on this it gives the warning datatable dt
is not disposed of in all exception paths.
How can I dispose of the datatable?
It is the very data that is being returned.
Click to expand...

You can not Dispose and return it.

The analysis tool is broken.

Arne
 
The analysis tool is not complaining about not disposing an object that's
being returned. It's complaining about not disposing it when it's _not_
returned.

Seems to me, that's a perfectly valid warning.
Click to expand...

If that is the case, then it is valid to give a warning and just
the warning text that should be a bit clearer.

Arne
 
It's not being returned when you throw an exception.

The CA warning should go away if you dispose "dt" before the "throw"
statement.

Oddly enough, the warning message is quite specific about this. That's
exactly what it means by "in all exception paths". It's not concerned about
the non-exception code path. Just the one where you throw an exception
instead of returning the DataTable instance.

Read all compiler messages carefully. They often have exactly the
information you need. :)

Pete
Click to expand...

OK, thanks.

What would be the case if I use a finally block?
If I add a finally block to dispose the object there, would the object
be returned when there is no exception?
 
Code in a "finally" block will always execute, whether the method succeeds
or not. You don't want to dispose in a "finally" block, because you _do_
want to return the object on success. Your method would still return the
object, but in that case it would always be disposed.

You really do just want to dispose the object just before the "throw"
statement (or more generally, before _any_ thrown exception in the
method...it just happens that in the code you posted, there's only one
place that happens, in the "catch" block). That will ensure it's disposed
on failure, but not on success when you're able to return the reference to
the object.

Pete
Click to expand...

One more question on using "using" block.
If I have an object, MyObject, that implements IDisposable would the
follwing code work?
That is it returns the object in case of success and dispose it in the
case of failure.


Try
[
using (MyObject o = new MyObject())
{
// do stuff here

return o;
}
}
catch
{
// handle exception
}
 
No. The whole point of a "using" block is that the object
declared/specified in the "using" statement itself is guaranteed to be
disposed before control exits the block. Using the example you posted, the
object "o" would _always_ be disposed, not just when an exception occurred.

In fact, the "using" block is implemented by the compiler as a
"try/finally" block. So for the exact same reason a "try/finally" would not
be appropriate, neither would "using".

Pete
Click to expand...

Often in my methods I return complex data in a datatable that I
construct in code.

Consider this very simple example:

private DataTable GetData()
{
DataTable dt = new DataTable();
dt.Columns.Add("id", typeof(int));

DataRow dr = dt.NewRow();
dr["id"] = 12345;
dt.Rows.Add(dr);

return dt;
}

The code analysis gives the warning that the object dt should be
disposed of.
How can it be disposed and at the same time return a valid object?

Yet when I tried the following code:

private DataTable GetData()
{
using (DataTable dt = new DataTable())
{
dt.Columns.Add("id", typeof(int));

DataRow dr = dt.NewRow();
dr["id"] = 12345;
dt.Rows.Add(dr);

return dt;
}
}

it wotked, in the sense that in the calling method the dt has correct
values. It doesn't seem disposed.

But in another example I made up my own object which implements
IDisposable.
When I try the above method, with "using", the object in the calling
method doesn't have the correct values (I reset some property in
the Dispose method).
So it seems the object has indeed been disposed.

How do you explain this?
Why would datatable work with "using", but not my custom object?
 
Often in my methods I return complex data in a datatable that I
construct in code.

Consider this very simple example:

private DataTable GetData()
{
DataTable dt = new DataTable();
dt.Columns.Add("id", typeof(int));

DataRow dr = dt.NewRow();
dr["id"] = 12345;
dt.Rows.Add(dr);

return dt;
}

The code analysis gives the warning that the object dt should be
disposed of.
How can it be disposed and at the same time return a valid object?
Click to expand...

It's hard to say without a proper concise-but-complete code example and
without the exact compiler message you are getting. However, in that
example there is nothing that ensures "dt" is disposed if an exception
occurs in any of the methods called by the "GetData()" method.

So the code analysis would be correct to object to the lack of a try/catch
in which you dispose the object. If you include a try/catch to deal with
exceptions, that should satisfy the code analysis:

private DataTable GetData()
{
DataTable dt = new DataTable();

try
{
dt.Columns.Add("id", typeof(int));

DataRow dr = dt.NewRow();
dr["id"] = 12345;
dt.Rows.Add(dr);
}
catch
{
dt.Dispose();
throw;
}


return dt;
}
Click to expand...

I usually don't have try/catch in my sub methods.
I let the exception "bubble up" and be handled at the top of
the food chain so to speeak.
Because catch then throw seems to me to be redundant.
Maybe that is not a good practice.

But you are right. The warning message invariably includes
the phrase "not in all exception paths".

So code analysis is mostly concerned about what happens
in case of an exception.

Thank you for this informative discussion.
 
Back
Top