Is it abusive to use IDisposable and “using” as a

Question

Something I often used back in C++ was letting a class A handle a state entry and exit condition for another class B, via the A constructor and destructor, to make sure that if something in that scope threw an exception, then B would have a known state when the scope was exited. This isn't pure RAII as far as the acronym goes, but it's an established pattern nevertheless.

In C#, I often want to do

class FrobbleManager
{
    ...

    private void FiddleTheFrobble()
    {
        this.Frobble.Unlock();
        Foo();                  // Can throw
        this.Frobble.Fiddle();  // Can throw
        Bar();                  // Can throw
        this.Frobble.Lock();
    }
}

Which needs to be done like this

private void FiddleTheFrobble()
{
    this.Frobble.Unlock();

    try
    {            
        Foo();                  // Can throw
        this.Frobble.Fiddle();  // Can throw
        Bar();                  // Can throw
    }
    finally
    {
        this.Frobble.Lock();
    }
}

if I want to guarantee the Frobble state when FiddleTheFrobble returns. The code would be nicer with

private void FiddleTheFrobble()
{
    using (var janitor = new FrobbleJanitor(this.Frobble))
    {            
        Foo();                  // Can throw
        this.Frobble.Fiddle();  // Can throw
        Bar();                  // Can throw
    }
}

where FrobbleJanitor looks roughly like

class FrobbleJanitor : IDisposable
{
    private Frobble frobble;

    public FrobbleJanitor(Frobble frobble)
    {
        this.frobble = frobble;
        this.frobble.Unlock();
    }

    public void Dispose()
    {
        this.frobble.Lock();
    }
}

And that's how I want to do it. Now reality catches up, since what I want to use requires that the FrobbleJanitor is used with using. I could consider this a code review issue, but something is nagging me.

Question: Would the above be considered as abusive use of using and IDisposable?

Answer 1

Eric Gunnerson, who was on the C# language design team, gave this answer to pretty much the same question:

Doug asks:

re: A lock statement with timeout...

I've done this trick before to deal with common patterns in numerous methods. Usually lock acquisition, but there are some others. Problem is it always feels like a hack since the object isn't really disposable so much as "call-back-at-the-end-of-a-scope-able".

Doug,

When we decided[sic] the using statement, we decided to name it “using” rather than something more specific to disposing objects so that it could be used for exactly this scenario.

Answer 2

Note: My viewpoint is probably biased from my C++ background, so my answer's value should be evaluated against that possible bias...

What Says the C# Language Specification?

Quoting C# Language Specification:

8.13 The using statement

[...]

A resource is a class or struct that implements System.IDisposable, which includes a single parameterless method named Dispose. Code that is using a resource can call Dispose to indicate that the resource is no longer needed. If Dispose is not called, then automatic disposal eventually occurs as a consequence of garbage collection.

The Code that is using a resource is, of course, the code starting by the using keyword and going until the scope attached to the using.

So I guess this is Ok because the Lock is a resource.

Perhaps the keyword using was badly chosen. Perhaps it should have been called scoped.

Then, we can consider almost anything as a resource. A file handle. A network connection... A thread?

A thread???

Using (or abusing) the using keyword?

Would it be shiny to (ab)use the using keyword to make sure the thread's work is ended before exiting the scope?

Herb Sutter seems to think it's shiny, as he offers an interesting use of the IDispose pattern to wait for a thread's work to end:

http://www.drdobbs.com/go-parallel/article/showArticle.jhtml?articleID=225700095

Here is the code, copy-pasted from the article:

// C# example
using( Active a = new Active() ) {    // creates private thread
       …
       a.SomeWork();                  // enqueues work
       …
       a.MoreWork();                   // enqueues work
       …
} // waits for work to complete and joins with private thread

While the C# code for the Active object is not provided, it is written the C# uses the IDispose pattern for the code whose C++ version is contained in the destructor. And by looking at the C++ version, we see a destructor which waits for the internal thread to end before exiting, as shown in this other extract of the article:

~Active() {
    // etc.
    thd->join();
 }

So, as far as Herb is concerned, it's shiny.

Answer 3

We have plenty of use of this pattern in our code base, and I've seen it all over the place before - I'm sure it must have been discussed here as well. In general I don't see what's wrong with doing this, it provides a useful pattern and causes no real harm.

Answer 4

I don't think so, necessarily. IDisposable technically is meant to be used for things that have non-managed resources, but then the using directive is just a neat way of implementing a common pattern of try .. finally { dispose }.

A purist would argue 'yes - it's abusive', and in the purist sense it is; but most of us do not code from a purist perspective, but from a semi-artistic one. Using the 'using' construct in this way is quite artistic indeed, in my opinion.

You should probably stick another interface on top of IDisposable to push it a bit further away, explaining to other developers why that interface implies IDisposable.

There are lots of other alternatives to doing this but, ultimately, I can't think of any that will be as neat as this, so go for it!

Answer 5

I believe the answer to your question is no, this would not be an abuse of IDisposable.

The way I understand the IDisposable interface is that you are not supposed to work with an object once it has been disposed (except that you're allowed to call its Dispose method as often as you want).

Since you create a new FrobbleJanitor explicitly each time you get to the using statement, you never use the same FrobbeJanitor object twice. And since it's purpose is to manage another object, Dispose seems appropriate to the task of freeing this ("managed") resource.

(Btw., the standard sample code demonstrating the proper implementation of Dispose almost always suggests that managed resources should be freed, too, not just unmanaged resources such as file system handles.)

The only thing where I'd personally worry is that it's less clear what's happening with using (var janitor = new FrobbleJanitor()) than with a more explicit try..finally block where the Lock & Unlock operations are directly visible. But which approach is taken probably comes down to a matter of personal preferences.

Answer 6

A real world example is the BeginForm of ASP.net MVC. Basically you can write:

Html.BeginForm(...);
Html.TextBox(...);
Html.EndForm();

or

using(Html.BeginForm(...)){
    Html.TextBox(...);
}

Html.EndForm calls Dispose, and Dispose just outputs the </form> tag. The good thing about this is that the { } brackets create a visible "scope" which makes it easier to see what's within the form and what not.

I wouldn't overuse it, but essentially IDisposable is just a way to say "You HAVE to call this function when you're done with it". MvcForm uses it to make sure the form is closed, Stream uses it to make sure the stream is closed, you could use it to make sure the object is unlocked.

Personally I would only use it if the following two rules are true, but thet are set arbitarily by me:

• Dispose should be a function that always has to run, so there shouldn't be any conditions except Null-Checks
• After Dispose(), the object should not be re-usable. If I wanted a reusable object, I'd rather give it open/close methods rather than dispose. So I throw an InvalidOperationException when trying to use a disposed object.

At the end, it's all about expectations. If an object implements IDisposable, I assume it needs to do some cleanup, so I call it. I think it usually beats having a "Shutdown" function.

That being said, I don't like this line:

this.Frobble.Fiddle();

As the FrobbleJanitor "owns" the Frobble now, I wonder if it wouldn't be better to instead call Fiddle on he Frobble in the Janitor?