19

If I use object initializers in using-block I get Code Analysis warning about not disposing the object properly:

CA2000 : Microsoft.Reliability : In method 'ReCaptcha.CreateReCaptcha(this HtmlHelper, string, string)', 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.

Here is the code:


    using (var control = new ReCaptchaControl()
    {
        ID = id,
        Theme = theme,
        SkipRecaptcha = false
    })
    {
        // Do something here
    }

If I do not use object initializers, Code Analysis is happy:


    using (var control = new ReCaptchaControl())
    {
        control.ID = id;
        control.Theme = theme;
        control.SkipRecaptcha = false; 

        // Do something here
    }

What is the difference between those two code blocks? I thought that they would result in same IL. Or is this a bug in the code analysis engine?

Tero
  • 272
  • 2
  • 7

1 Answers1

33

No, there's a difference.

An object initializer only assigns to the variable after all the properties have been set. In other words, this:

Foo x = new Foo { Bar = "Baz" };

is equivalent to:

Foo tmp = new Foo();
tmp.Bar = "Baz";
Foo x = tmp;

That means that if one of the property setters threw an exception in your case, the object wouldn't be disposed.

EDIT: As I thought... try this:

using System;

public class ThrowingDisposable : IDisposable
{
    public string Name { get; set; }

    public string Bang { set { throw new Exception(); } }

    public ThrowingDisposable()
    {
        Console.WriteLine("Creating");
    }

    public void Dispose()
    {
        Console.WriteLine("Disposing {0}", Name);
    }
}

class Test
{
    static void Main()
    {
        PropertiesInUsingBlock();
        WithObjectInitializer();
    }

    static void PropertiesInUsingBlock()
    {
        try
        {
            using (var x = new ThrowingDisposable())
            {
                x.Name = "In using block";
                x.Bang = "Ouch";
            }
        }
        catch (Exception)
        {
            Console.WriteLine("Caught exception");
        }
    }

    static void WithObjectInitializer()
    {
        try
        {
            using (var x = new ThrowingDisposable
            {
                Name = "Object initializer",
                Bang = "Ouch"
            })
            {
                // Nothing
            }
        }
        catch (Exception)
        {
            Console.WriteLine("Caught exception");
        }
    }
}

Output:

Creating
Disposing In using block
Caught exception
Creating
Caught exception

Note how there's no "Disposing Object initializer" line.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 1
    That makes sense now that you write it. IMO this is a pitfall that is easily overlooked. Luckily Visual Studio is wiser than me. – Tero Aug 18 '10 at 17:57
  • 1
    @Jon - So is it right to conclude that 'Do not use object initializer syntax with a type that also implements IDisposable'. (Since the expansion isn't in our control to correct the issue flagged by the CA engine.) – Gishu Jan 21 '14 at 12:15
  • 2
    @Gishu: If you're at all concerned that the property setters might throw exceptions, yes. – Jon Skeet Jan 21 '14 at 13:34
  • Very good reduction of the problem. Thank you. I agree with the earlier post: I'm glad FXCop is smarter than I am. I would have overlooked this. – Quark Soup Feb 21 '15 at 16:51