Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

IDisposable: is it necessary to check for null on finally {}?

In most examples that you find on the web when explicitly not using "using", the pattern looks something like:

  SqlConnection c = new SqlConnection(@"...");
  try {
    c.Open();
  ...
  } finally {
    if (c != null) //<== check for null
      c.Dispose();
  }

If you do use "using" and look at the generated IL code, you can see that it generates the check for null

L_0024: ldloc.1 
L_0025: ldnull 
L_0026: ceq 
L_0028: stloc.s CS$4$0000
L_002a: ldloc.s CS$4$0000
L_002c: brtrue.s L_0035
L_002e: ldloc.1 
L_002f: callvirt instance void [mscorlib]System.IDisposable::Dispose()
L_0034: nop 
L_0035: endfinally 

I understand why the IL gets translated to check for null (doesn't know what you did inside the using block), but if you're using try..finally and you have full control of how the IDisposable object gets used inside the try..finally block, do you really need to check for null? if so, why?

like image 660
BlackTigerX Avatar asked Apr 08 '10 05:04

BlackTigerX


2 Answers

"using" statements can initialize variables with calls other than constructors. For example:

using (Foo f = GetFoo())
{
    ...
}

Here f could easily be null - whereas a constructor call can never1 return null. That's why a using statement checks for nullity. It's not to do with what's inside the block itself, because the using statement preserves the original initial value. If you write:

Stream s;
using (s = File.OpenRead("foo.txt"))
{
    s = null;
}

then the stream will still be disposed. (If the variable is declared in the initializer part of the using statement, it's read-only anyway.)

In your case, as you know that c is non-null before you enter the try block, you don't need to check for null in the finally block unless you're reassigning its value (which I sincerely hope you're not!) within the block.

Now with your current code there is the slight risk that an asynchronous exception could be thrown between the assignment to c and entering the try block - but it's hard to avoid this sort of race condition completely, as there could equally be an asynchronous exception after the constructor has completed but before the value is assigned to c at all. I would suggest that most developers don't need to worry about this sort of thing - asynchronous exceptions tend to be sufficiently "hard" that they'll bring down the process anyway.

Is there any reason you don't want to just use a using statement anyway though? To be honest, I very rarely write my own finally blocks these days...


1 See Marc's answer and weep. Not usually relevant though.

like image 59
Jon Skeet Avatar answered Nov 06 '22 19:11

Jon Skeet


In your example, the check for null is unnecessary, as c can't be null after explicit construction.

In the following example (similar to what is generated by the using statement), a check for null would of course be necessary:

SqlConnection c = null; 
  try { 
    c = new SqlConnection(@"..."); 
    c.Open(); 
  ... 
  } finally { 
    if (c != null) //<== check for null 
      c.Dispose(); 
  } 

In addition, a check for null would be required in the following case, as you can't be sure CreateConnection won't return null:

SqlConnection c = CreateConnection(...); 
  try { 
    c.Open(); 
  ... 
  } finally { 
    if (c != null) //<== check for null 
      c.Dispose(); 
  } 
like image 26
Joe Avatar answered Nov 06 '22 18:11

Joe