3

TL;DR -- Is it ever appropriate to execute business logic in IDisposable.Dispose?

In my search for an answer, I read through the question: Is it abusive to use IDisposable and "using" as a means for getting "scoped behavior" for exception safety? It came very close to addressing this issue, but I'd like to attack it dead on. I recently encountered some code that looked like this:

class Foo : IDisposable
{
    public void Dispose()
    {
        ExecuteSomeBusinessBehavior();
        NormalCleanup();
    }
}

and is used in a context such as:

try 
{
    using (var myFoo = new Foo())
    {
        DoStuff();
        foo.DoSomethingFooey();
        ...
        DoSomethingElse();
        Etc();
    }
}
catch (Exception ex)
{
    // Handle stuff
}

Upon seeing this code I immediately began to itch. Here's what I see when I look at this code:

First, looking at just the usage context, it's not remotely apparent that actual business logic, not just cleanup code, will be executed when the code leaves the using scope.

Second, if any of the code within the "using" scope throws an exception, the business logic in the Dispose method will still execute and does so before the Try/Catch can handle the exception.

My questions to the StackOverflow community are these: Does it ever make sense to put business logic in the IDisposable.Dispose method? Is there a pattern that achieves similar results without making me itch?

4

4 回答 4

5

(Sorry, this is more of a comment, but it exceeds the comment length limit.)

Actually, there is an example in the .NET framework where IDisposable is used to create a scope and do useful work when disposing: TransactionScope.

To quote from TransactionScope.Dispose:

Calling this method marks the end of the transaction scope. If the TransactionScope object created the transaction and Complete was called on the scope, the TransactionScope object attempts to commit the transaction when this method is called.

If you decide to take that route, I would suggest that

  • you make it blatantly obvious that your object creates a scope, e.g., by calling it FooScope instead of Foo and

  • you think very hard about what should happen when an exception causes the code to leave your scope. In TransactionScope, the pattern of calling Complete at the end of the block ensures that Dispose can distinguish between the two cases.

于 2013-06-26T16:38:37.237 回答
3

The real meaning of IDisposable is that an object knows of something, somewhere which has been put into a state that should be cleaned up, and it has the information and impetus necessary to perform such cleanup. Although the most common "states" associated with IDisposable are things like files being open, unmanaged graphic objects being allocated, etc. those are only examples of uses, and not a definition of "proper" use.

The biggest issue to consider when using IDisposable and using for scoped behavior is that there is no way for the Dispose method to distinguish scenarios where an exception is thrown from a using block from those where it exits normally. This is unfortunate, since there are many situations where it would be useful to have scoped behavior which was guaranteed to have one of two exit paths depending upon whether the exit was normal or abnormal.

Consider, for example, a reader-writer lock object with a method that returns an IDisposable "token" when the lock is acquired. It would be nice to say:

using (writeToken = myLock.AcquireForWrite())
{
   ... Code to execute while holding write lock
}

If one were to manually code the acquisition and release of the lock without a try/catch or try/finally lock, an exception thrown while the lock was held would cause any code that was waiting on the lock to wait forever. That is a bad thing. Employing a using block as shown above will cause the lock to be released when the block exits, whether normally or via exception. Unfortunately, that may also be a bad thing.

If an unexpected exception is thrown while a write-lock is held, the safest course of behavior would be to invalidate the lock so that any present or future attempt to acquire the lock will throw an immediate exception. If the program cannot usefully proceed without the locked resource being usable, such behavior would cause it to shut down quickly. If it can proceed e.g. by switching to some alternate resource, invalidating the resource will allow it to get on with that much more effectively than would leaving the lock uselessly acquired. Unfortunately, I don't know of any nice pattern to accomplish that. One could do something like:

using (writeToken = myLock.AcquireForWrite())
{
   ... Code to execute while holding write lock
   writeToken.SignalSuccess();
}

and have the Dispose method invalidate the token if it's called before success has been signaled, but an accidental failure to signal the success could cause the resource to become invalid without offering indication as to where or why that happened. Having the Dispose method throw an exception if code exits a using block normally without calling SignalSuccess might be good, except that throwing an exception when it exits because of some other exception would destroy all information about that other exception, and there's no way Dispose can tell which method applies.

Given those considerations, I think the best bet is probably to use something like:

using (lockToken = myLock.CreateToken())
{
   lockToken.AcquireWrite(Describe how object may be invalid if this code fails");
   ... Code to execute while holding write lock
   lockToken.ReleaseWrite();
}

If code exits without calling ReleaseWrite, other threads that try to acquire the lock will receive exceptions that include the indicated message. Failure to properly manually pair the AcquireWrite and ReleaseWrite will leave the locked object unusable, but not leave other code waiting for it to become usable. Note that an unbalanced AcquireRead would not have to invalidate the lock object, since code inside the read would never put the object into an invalid state.

于 2013-06-26T17:00:23.100 回答
2

Business logic code should never be written in any circumstances to Dispose method reason is, you are relying on a unreliable path. What if user does not call your dispose method? You missed to call a complete functionality ? What if there was an exception thrown in the method call of your dispose method? And why would you perform a business operation when user is asking to dispose the object itself. So logically, technically it should not be done.

于 2013-06-26T13:29:38.927 回答
0

I'm currently reading Introduction to Rx, by Lee Campbell, and it has a chapter called IDisposable, where he explicitly advocates taking advantage of the integration with the using construct, in order to "create transient scope".

Some key quotations from that chapter:

"If we consider that we can use the IDisposable interface to effectively create a scope, you can create some fun little classes to leverage this."

(...see examples below...)

"So we can see that you can use the IDisposable interface for more than just common use of deterministically releasing unmanaged resources. It is a useful tool for managing lifetime or scope of anything; from a stopwatch timer, to the current color of the console text, to the subscription to a sequence of notifications.

The Rx library itself adopts this liberal usage of the IDisposable interface and introduces several of its own custom implementations:

  • BooleanDisposable
  • CancellationDisposable
  • CompositeDisposable
  • ContextDisposable
  • MultipleAssignmentDisposable
  • RefCountDisposable
  • ScheduledDisposable
  • SerialDisposable
  • SingleAssignmentDisposable"

He gives two fun little examples, indeed:

Example 1 - Timing code execution. "This handy little class allows you to create scope and measure the time certain sections of your code base take to run."

public class TimeIt : IDisposable
{
    private readonly string _name;
    private readonly Stopwatch _watch;

    public TimeIt(‌string name)
    {
        _name = name;
        _watch = Stopwatch‌.StartNew(‌);
    }

    public void Dispose(‌)
    {
        _watch‌.Stop(‌);
        Console‌.WriteLine(‌"{0} took {1}", _name, _watch‌.Elapsed);
    }
}

using (‌new TimeIt(‌"Outer scope"))
{
    using (‌new TimeIt(‌"Inner scope A"))
    {
        DoSomeWork(‌"A");
    }

    using (‌new TimeIt(‌"Inner scope B"))
    {
        DoSomeWork(‌"B");
    }

    Cleanup(‌);
}

Output:

Inner scope A took 00:00:01.0000000
Inner scope B took 00:00:01.5000000
Outer scope took 00:00:02.8000000

Example 2 - Temporarily changing console text color

//Creates a scope for a console foreground color‌. When disposed, will return to 
//  the previous Console‌.ForegroundColor

public class ConsoleColor : IDisposable
{
    private readonly System‌.ConsoleColor _previousColor;
    
    public ConsoleColor(‌System‌.ConsoleColor color)
    {
        _previousColor = Console‌.ForegroundColor;
        Console‌.ForegroundColor = color;
    }

    public void Dispose(‌)
    {
        Console‌.ForegroundColor = _previousColor;
    }
}


Console‌.WriteLine(‌"Normal color");

using (‌new ConsoleColor(‌System‌.ConsoleColor‌.Red))
{
    Console‌.WriteLine(‌"Now I am Red");

    using (‌new ConsoleColor(‌System‌.ConsoleColor‌.Green))
    {
        Console‌.WriteLine(‌"Now I am Green");
    }

    Console‌.WriteLine(‌"and back to Red");
}

Output:

Normal color
Now I am Red
Now I am Green
and back to Red
于 2017-01-06T13:51:22.167 回答