Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Object initializer and Dispose when property can throw exception

Tags:

c#

I've got the following setup:

public class SomeClass
{
  private DirectoryEntry _root;
  private DirectorySearcher _searcher;

  public SomeClass()
  {
    _root = new DirectoryEntry("ldap://bla");
    _searcher = new DirectorySearcher(_root)
      {
        PageSize = int.MaxValue,
        SizeLimit = int.MaxValue
      }
  }
}

The reason I use int.MaxValue is because in this case I know I'll exceed the defaults, but the number will never be ridiculously large, so I'm fine about that point.

But if I turn on Code Analysis and Microsoft Basic Correctness Rules it complains:

Warning 2 CA2000 : Microsoft.Reliability : In method 'SomeClass.SomeClass()', object '<>g_initLocal0' is not disposed along all exception paths. Call System.IDisposable.Dispose on object '<>g_initLocal0' before all references to it are out of scope.

It's problem is that PageSize and SizeLimit can throw exceptions, and if that happens the G__initLocal0 object doesn't get disposed (even if _searcher does get disposed). The exception they can throw is if you assign them to a negative number, which can't happen here but it still complains.

Next I set the properties outside an object intitializer with regular assignment statements, but then ReSharper complains telling me that I should be using Initializer. I could suppress ReSharper, but I like to figure a way of getting things to work without adding suppressing.

So I figured I had to catch the error and I don't like catch errors in my constructors if possible, so I created a property called Searcher like this:

private DirectorySearcher _searcher;
public DirectorySearcher Searcher
{
  get
  {
    if (_searcher != null) return _searcher;
    var searcher = new DirectorySearcher();
    try
    {
      searcher.PageSize = int.MaxValue;
      searcher.SizeLimit = int.MaxValue;
      _searcher = searcher;
    }
    catch
    {
      searcher.PageSize = 1000;
      searcher.SizeLimit = 1000;
    }
    finally
    {
      searcher.Dispose();
    }
    return _searcher;
  }
}

Now Code Analysis and everything is happy, but I'm not happy with the solution at all.

Any hints?

like image 911
carlsb3rg Avatar asked Jun 20 '11 10:06

carlsb3rg


2 Answers

The way you are doing things now might make everyone happy but you as it wont work. You are returning a disposed DirectorySearcher in your Searcher property.

I'd just do:

public SomeClass()
{
    _root = new DirectoryEntry("ldap://bla");

    try
    {
        _searcher = new DirectorySearcher(_root);
        _searcher.PageSize = 1000;
        _searcher.SizeLimit = 1000;
    }
    catch
    {
         if (_searcher != null)
         {
             _searcher.Dispose();
         }

         throw;
    }

}

I don't see whats wrong in using try-catch blocks in constructors.

I wouldn't recommed the property initializer syntax when constructing IDisposable objects as you cannot dispose them normally if a property initialization throws.

like image 39
InBetween Avatar answered Oct 23 '22 17:10

InBetween


The problem is the compiler is effectively generating:

public class SomeClass
{
  private DirectoryEntry _root;
  private DirectorySearcher _searcher;

  public SomeClass()
  {
    _root = new DirectoryEntry("ldap://bla");

    var temp = new DirectorySearcher(_root);
    temp.PageSize = int.MaxValue;
    temp.SizeLimit = int.MaxValue;

    _searcher = temp;
  }
}

You can avoid this by not using the newer property initializer syntax for _searcher, so that you can ensure it is properly assigned to the field before you set the properties, see Object initializers in using-block generates code analysis warning CA2000.

There is a second problem here, and that is if there is an error during construction of SomeClass the calling code will not be able to dispose of SomeClass and thus cannot dispose of _root or _searcher, see Handling IDisposable in failed initializer or constructor.

like image 67
Chris Chilvers Avatar answered Oct 23 '22 16:10

Chris Chilvers