Pattern based using

Topics: C# Language Design
Apr 14, 2014 at 10:18 AM
Hi,

I would like to have using statement which would not require particular type to implement IDisposable. Just provide "public void Close()" or other method which will be picked by its signature. Such statement would allow to use succint syntax instead of full try {} finally {} verbosity.

As "canonical" example consider:
class Timer
{
    string operation;
    Stopwatch s;
    public Timer(string operation)
    {
        this.operation = operation;
        s = Stopwatch.StartNew();
    }
    public void Close()
    {
        s.Stop();
        Console.WriteLine("{0} took {1} milliseconds", operation, s.ElapsedMilliseconds);
    }
}
And consuming code:
      using (new Timer("Foo"))
      {
        // do foo
      }
The reason I don't want my Timer class to implement IDisposable is all the GC overhead it brings to the table. If I need to implement whole IDisposable pattern, it adds lots of unnecessary and unrelated code to my simple class.

Other constructs like async + await, LINQ and even foreach work by method inference and do not require implementation of specific interface (though it is recommended to use IEnumerable<T> in case of foreach). Ok, I admit this argument is actually quite weak - LINQ and async await are to complex to use interface and foreach would require IEnumerable<T> if only it could be enforced from the beginning. But still.

And if not match by signature, then maybe introduce some other interface which would be accepted by using statement, but which would not be connected to GC mechanics?

Thoughts?
Apr 14, 2014 at 11:15 AM
I don't want my Timer class to implement IDisposable is all the GC overhead it brings to the table.
IDisposable has nothing to do with GC so it doesn't add any GC overhead.
If I need to implement whole IDisposable pattern, it adds lots of unnecessary and unrelated code to my simple class.
If the class is sealed or private/internal then there's no real need to implement the whole IDisposable pattern. That pattern solely exists to ensure that derived classes can be disposed properly. If the class is sealed or if you know that the only derived class are implemented by yourself then the need for the Dispose pattern disappears.
Apr 14, 2014 at 1:13 PM
If we don't implement full IDisposable pattern, then you're right - it has no impact on GC. However, as soon as finalizer is added, things change. I agree, that in the cases we discuss here, there are no real resources to free, so there is no need to use finalizer and full pattern at all.
But - you never know what need arise in derived classes. And we don't want restrict feature to sealed or private/internal etc. We want it to work in general. If we hijack Dispose() method just to be able to use using statement for things unrelated to actual disposal, then problems might show up in derived types, which might require disposing something. Also, even in internal class scenario, it might be tempting for other devs to "fix" the class by adding full pattern implementation unnecessary.

IDisposable has its meaning. Very important one. In my opinion it should not be abused just to gain syntactic wins. Hence the idea - to disconnect "using" statement syntax from disposing resources.
Apr 14, 2014 at 2:33 PM
Przemyslaw wrote:
However, as soon as finalizer is added, things change.
The dispose pattern doesn't require you to add a finalizer. A finalizer should be added only if it is needed.
IDisposable has its meaning. Very important one. In my opinion it should not be abused just to gain syntactic wins. Hence the idea - to disconnect "using" statement syntax from disposing resources.
That's kind of true but people are doing this already, it sort of became the norm.
May 2, 2014 at 12:11 AM
I was having a thought about this today also. I really enjoy using the using(IDisposable){ } pattern to enforce scope over certain operations. It, by my reckoning, helps show intent and denotes when an operation is valid aka in scope. But I've also had some negative feelings about hijacking the IDisposable interface for these syntactic "gains" also. I've seen Eric Lippert speak out against this before. (Google "c# using abuse" for related chatter about this topic). But for every purist response that it's a syntax abuse, i see several others saying "well yes it is an abuse, but golly is it useful". (See also post by Eric Gunnerson.)


What if the framework could provide a second type, IScopedOperation (the best name 10 seconds of thinking could get me) that was also legal in using blocks. Instead of dispose(), it could have a Complete() method. We could use the same try/finally code transform to implement it. It sounds very low effort, the only concern is about polluting the language with more complexity. But for this small change i would very much think its worthwhile.

Thinking about it a bit more there are some other things we could do here to support the pattern. Feel free to pick and choose pieces ideas if any of these are to nutty.
  1. Give IScopedOperation a void Start() and void End() instead of just a Complete(). The call to Start() is inserted just inside of the compiler generated try block and the call to end is placed in the finally block. This would allow you to pass in a pre-existing IScopedOperation or reuse a IScopedOperation in multiple using blocks for in loops to avoid overhead of reconstruction. But that job is already mostly solved by the constructor, so the Start() method might be of limited utility.
  2. Same as 1 but also give IScopedOperation a void Catch(Exception ex) method that could get called by a catch block to allow generic exception processing code (logging, wrapping, settings faulted states...) to be "passed around" more handily. This is already possible with various ReThrowFormattedException(Exception ex), WrapException(Exception ex) and related patterns that we see in C# code today, but it avoids the slight verbosity of having to explicitly write the try,catch,finally pattern directly. 2 things to note, A) I have some vague concerns about certain types of exceptions not being safe for this pattern. and B) this would also introduce the expense of an allocation just for exception handling even in non-exceptional cases. This second point could be combated by retaining the IScopedOperation in the enclosing class's state or as a static object (is encouraging this an idea evil)?.
  3. Same as 2 except also give a bool Failed {get; set;} Property to the IScopedOperation type. The idea here is that if an exception is thrown then we set the Failed property to true, then call the aforementioned "void Catch(Exception ex)". This would make the class even more useful for making exception handling more portable in code. Furthermore, the Failed property could be set manually inside of the using block to indicate to the IScopedOperation that the operation has failed in some non-exception based manner. The implementor of the IScopedOperation should inspect Failed inside of the End() method body to see if the operation needs to be rolled back, or if some other remedial action needs to be taken.
  4. Same as 3 except also overload the break keyword to escape from the using block and immediately call the End() method as a short circuit. If this bit is implemented then i would also want to push for a break break; syntax to break nested loops and using blocks. In fact i might open a separate discussion thread for this syntax alone since it would be helpful for existing nested loops.
I think that some of these ideas might be useful for promoting this pattern while mitigating the abuse of the IDisposable interface. The concepts could be split into two separate interfaces also, but that would probably just complicate the matter unnecessarily. If anybody would like me to i could post sample syntax to clarify any points.
May 6, 2014 at 2:07 PM
There are a number of cases where resource cleanup should throw an exception if none is pending, but should not cause any pending exception to be destroyed; even if the resource cleanup failure represents sufficiently dire emergency that it should be the primary exception thrown to the caller, it should encapsulate any exception which was pending when the cleanup was attempted. Consequently, I would suggest that the best way to allow that would be an IDisposableEx interface with a Dispose(Exception Ex) method. If interfaces were set up as:
Interface IDisposableExOnly { void Dispose(Exception pendingException;}
Interface IDisposableEx : IDisposableExOnly, IDisposable {}
and using would use IDisposableExOnly for types which implement it [pass null for pendingException in the "success" case], that would not only solve the exception-on-cleanup problem, but it would also be workable for transaction scopes. The only thing that might be lacking would be a distinction between cases where a scoped block was exit via return versus fall-through. While a parameter could be added for that, the only real use I can see would be to throw an exception if code returned from a scoped block which was only supposed to exit via fall-through, and that scenario could be better handled via e.g. a "using noreturn(...) {}` syntax.
May 6, 2014 at 2:49 PM
Interesting although we are arguing for 2 slightly different things here. Myself and Przemyslaw are attempting to avoid the use of IDisposable for types that either do not posses resources to clean up, but still retain the using pattern for non-disposable operations.

If I've read correctly, you (supercat) are advocating for and extension to the IDisposable interface that would get called in the event of an exception being thrown. Basically a catch block. I'm not opposed to this and actually think it is a good idea since it allows exception handling code to be reused more effectively. For simplicity's sake i would probably only add a single interface: IDisposableEx : IDisposable { void Catch(Exception pendingException); }.

This require 2 explanations :
  1. The reason i changed the name of the method is to map the basic flow of the try-catch-finally pattern with this new-fangled using-catch-dispose pattern. The catch method would only be called in the event of an exception inside of the using block, but i would argue that the Dispose method should still be called even after catch has been called, similar to the behavior of a finally block. Therefor I've suggested the your Dispose(Exception) method be renamed to support that premise. What to you think?
  2. I would prefer an interface that inherits from IDisposable to allow simple conversion of existing code. However the IDisposableExOnly interface i don't think adds a great deal of value (i'm open to disagreement) since i think most scenarios would want to have both methods there and writing a no-op Dispose isn't difficult or overly harmful.
Those things being said I would still like to have a interface that is unrelated to IDisposable that could be used inside of the using block to keep the notion that all IDisposable types should always be disposed at the risk of some form of resource leak as opposed to some scopedOperation type that is designed as such for convenience.
May 6, 2014 at 3:21 PM
AlgorithmsAreCool wrote:
  1. The reason i changed the name of the method is to map the basic flow of the try-catch-finally pattern with this new-fangled using-catch-dispose pattern. The catch method would only be called in the event of an exception inside of the using block, but i would argue that the Dispose method should still be called even after catch has been called, similar to the behavior of a finally block. Therefor I've suggested the your Dispose(Exception) method be renamed to support that premise. What to you think?
I dislike Catch, since exceptions should only be caught by code which can resolve the underlying condition, and Dispose(Exception) would have no realistic hope of doing that. Further, in many usage cases, the vast majority of the cleanup code won't care about whether an exception was pending. Using one method with a parameter to indicate whether an exception occurred would avoid duplication of the common code.
  1. I would prefer an interface that inherits from IDisposable to allow simple conversion of existing code. However the IDisposableExOnly interface i don't think adds a great deal of value (i'm open to disagreement) since i think most scenarios would want to have both methods there and writing a no-op Dispose isn't difficult or overly harmful.
The reason I split the interface is to allow for the possibility of types where an old-style using block would yield undesirable semantics. For example, a lock-guarded object have previously required code like:
try {
  thing.AcquireWriteLock();
  thing.EnterDanger();
  ... do stuff that may render guarded object temporary invalid
  thing.LeaveDanger();
}
finally {
  thing.ReleaseWriteLock();
}
That could helpfully be replaced by:
using (thing.GetWriteToken())   // or perhaps "using noreturn (thing.GetWriteToken())"
{
  ... do stuff that may render guarded object temporarily invalid
}
but such replacement would only be safe if the Dispose method could know whether or not it should implicitly LeaveDanger.
Those things being said I would still like to have a interface that is unrelated to IDisposable that could be used inside of the using block to keep the notion that all IDisposable types should always be disposed at the risk of some form of resource leak as opposed to some scopedOperation type that is designed as such for convenience.
Why do you see scope guards as an exception to the principle that IDisposable objects should always given be disposed, given that they--just like other `IDisposable objects--should always be disposed?
May 12, 2014 at 8:45 PM
You could use a generic helper like the following:
class Disposable<T> : IDisposable
{
    public readonly T Value;

    readonly Action<T> InnerDispose;

    public Disposable(Func<T> create, Action<T> dispose)
    {
        InnerDispose = dispose;

        Value = create();
    }

    public void Dispose()
    {
        InnerDispose(Value);
    }
}
Then you could use it like the following:
using (new Disposable<Timer>(() => new Timer("Foo"), value => value.Close()))
{
    // do foo
}
May 12, 2014 at 10:43 PM
Hmm, drowa's pattern would work but i don't think that gains us anything over what we have today.

As to supercat :
Why do you see scope guards as an exception to the principle that IDisposable objects should always given be disposed, given that they--just like other `IDisposable objects--should always be disposed?
Although this point seems to ring true with me, i would cite Przemyslaw's explanation :
I agree, that in the cases we discuss here, there are no real resources to free, so there is no need to use finalizer and full pattern at all.
But - you never know what need arise in derived classes. And we don't want restrict feature to sealed or private/internal etc. We want it to work in general. If we hijack Dispose() method just to be able to use using statement for things unrelated to actual disposal, then problems might show up in derived types, which might require disposing something. Also, even in internal class scenario, it might be tempting for other devs to "fix" the class by adding full pattern implementation unnecessary.

IDisposable has its meaning. Very important one. In my opinion it should not be abused just to gain syntactic wins. Hence the idea - to disconnect "using" statement syntax from disposing resources.
If though there is no denotative distinction between any IDisposable type then every type that inherits IDisposable should implement the finalize pattern to ensure no leak of resources and all classes that hold references to IDisposables become IDisposable themselves. I would still prefer not to conflate this important concept with something like more frivolous like performance timers.

Essentially if an item if an item is IDispoable should we not always implement the finalizer pattern. If not then how do we know when it is essential? If not then how do we make use of the pattern for anything but critical resources without incurring performance and code overhead?
May 12, 2014 at 11:47 PM
Let's not forget why we have IDisposable in the first place; the lack of deterministic finalization. It is a burden imposed by the limitations of the GC.
May 13, 2014 at 12:43 AM
AlgorithmsAreCool wrote:
But - you never know what need arise in derived classes. And we don't want restrict feature to sealed or private/internal etc. We want it to work in general. If we hijack Dispose() method just to be able to use using statement for things unrelated to actual disposal, then problems might show up in derived types, which might require disposing something. Also, even in internal class scenario, it might be tempting for other devs to "fix" the class by adding full pattern implementation unnecessary.

If though there is no denotative distinction between any IDisposable type then every type that inherits IDisposable should implement the finalize pattern to ensure no leak of resources and all classes that hold references to IDisposables become IDisposable themselves. I would still prefer not to conflate this important concept with something like more frivolous like performance timers.
Outside of a few special circumstances, Finalize should not be viewed as anything more than a backstop that may in some cases manage to avoid some of the harm caused by resource leaks. In the absence of contracts which require otherwise, factory methods and constructors are expected to abide by the following contract:
  1. If the requested type of the instance (e.g. the declared type of the factory method) does not implements IDisposable, the created object may be abandoned at will.
  2. If the "requested" type of the instance does implement IDisposable, the code requesting its creation is expected to ensure that Dispose will get called on it once it's no longer needed, before it's abandoned; the object and associated responsibility may be handed off to other code, if such hand-off is specified by the contract of the means by which other code receives the objects and responsibility.
Essentially if an item if an item is IDispoable should we not always implement the finalizer pattern. If not then how do we know when it is essential? If not then how do we make use of the pattern for anything but critical resources without incurring performance and code overhead?
Any code which acquires responsibility for an IDisposable object and doesn't dispose it is broken. If a method which is supposed to call Dispose on something doesn't, and resources leak, the leak is the fault of the method which failed to call Dispose.

Besides, what is the meaning of an object "holding a resource", beyond the fact that something somewhere will be kept in a state which is less than optimal from the standpoint of the universe at large, until such time as that object allows the thing to be put into a better state? Such a description applies to many kinds of scope guards just as well as to other kinds of resources.
May 13, 2014 at 9:35 AM
Edited May 13, 2014 at 9:39 AM
Here is another argument why to split scoped operations and disposal of resources:

IDisposable is "technical" interface. There is a lot going in language design to help devs with correct and easy implementation of such interafces. Think how yield helped with IEnumerables. The ultimate goal I see is to instruct compiler with keywords/attributes/other annotations and have it to generate proper interface implementation for us. E.g. I would like to be able to mark some fields and properties and have Equals and GetHashCode autogenerated for me. C# is able to provide decent implementations of these methods - they do it for anonymous types already. I can see the need to automatically create code for various IEquatable<T>, IComparable<T>, INotifyPropertyChanged etc. So that we no longer need to "pollute" class with "technical", tedious stuff. And yes, I would like auto generated IDisposable as well. See Implicit using block and sample provided by bradphelan:
class Subscription : IDisposable 
{
    using IDisposable InnerResourceA;
    using IDisposable InnerResourceB;
    using IDisposable InnerResourceC;   
}
Compiler should be able to generate proper Dispose method which disposes above 3 fields. And maybe in the future compiler will be able to determine that Subscription object "owns" instances stored in these fields and even "using" annotations will be unnecessary. Anyway, in the ideal world of future c# I will never (or at least very rarely) see manually implemented Dispose method.

Obviously, compiler will not be able to auto generate this for scoped operations. Hence the need to split the two. In the edge case where scoped operation class has also resources, compiler can lower using statement to actually call both methods - IDisposable.Dispose() and IScopedOperetion.End(). Or, even better, auto generated IDisposable.Dispose() method could call IScopedOperetion.End() if necessary.
May 13, 2014 at 12:15 PM
Przemyslaw wrote:
Obviously, compiler will not be able to auto generate this for scoped operations.
Actually, the compiler does auto-generate IDisposable.Dispose for some scoped operations. If one writes an iterator that includes a finally block, the code within the finally block will execute when Dispose is called on the enumerator.

As for auto-generating things like IEquatable<T>, that would be possible if the type system could distinguish between different kinds of references, but there's no standard convention for doing so.

My main point is that if there isn't really anything different a compiler would want to do with a scoping interface than with IDisposable, there's really no need for using a different interface; indeed, there are some other things that would be more worthy of a split [e.g. if I had by druthers, there would be a foreach...from as well as a foreach...in, with the former representing a consuming enumeration operation suitable for use with things like queues, but I know of no such feature].