Implicit Dispose Pattern in C# (former Explicit Finalization in CLR)

Topics: C# Language Design
May 13, 2014 at 8:20 PM
Edited May 14, 2014 at 1:09 AM
I propose the creation of the delete operator as an alternative to the IDisposable interface. In this scenario IDisposable doesn't exist. For example, instead of doing this:
obj.Dispose();
We would do this:
delete obj;
Following the semantics of the delete operator:
  • When object is null, delete is a no op.
  • When object is not null, delete performs the following:
    1. calls Object.Finalize (GC won't call it again).
    2. effectively makes all references to the object (from the mutators) equal to null.
The using statement would be a syntactic sugar for delete in the same way it is now for IDisposable.Dispose.

Note if you want to propagate the delete operation down to object's members then you have to explicitly delete the members from the implementation of Object.Finalize.

Also note you wouldn't be able to use delete on value types, however you would be able to use it on any reference type.

Since IDisposable is so ingrained everywhere, I'm aware that something like the delete operator won't be approve. I'm just proposing this just to start a "what if" kind of discussion because for me IDisposable seems like a hack.

I left details (like multithread issues) for later during the discussion, if people find the idea interesting.
May 13, 2014 at 9:10 PM
First of all, the only objects should use Finalize for cleanup are generally those whose whole reason for being centers around finalization cleanup. An object of any significant complexity which would hold something that is supposed to be cleaned up when abandoned should should generally, rather than implementing Finalize itself, hold a reference to a finalizable object encapsulating the thing needing cleanup. Consequently, I would consider the proper way for a language to support Finalize would simply be to regard it as any other protected virtual method.

Secondly, while it would be useful to have a kind of declaration for "owned" IDisposable fields, and have the compiler auto-generate methods to facilitate cleanup, compilers would also have to deal with the fact that inheritable classes are not entirely consistent in how they implement IDisposable (some of them try to fix defects in Microsoft's recommended implementation), so it's not exactly clear what auto-generated code should do in all inheritance scenarios. My personal inclination would be to have the compiler either generate a default Dispose implementation if none exists, or else generate a method with a certain name that must be called from a user-written Dispose method (failure to have at least one call to a non-empty auto-generated dispose method would be an error).

Note that the mere fact that a field's type implements IDisposable does not imply that the holder of that field has any duty to call Dispose upon it. If multiple references exist to an IDisposable object, it's generally important to ensure that only one of them is responsible for calling Dispose upon it, and that such a disposal call doesn't happen until everyone else has finished using it. In such a situation, objects other than the designated owner must not call Dispose upon the things to which they hold references.
May 13, 2014 at 11:17 PM
Edited May 13, 2014 at 11:23 PM
What I'm really looking for is built-in support for deterministic finalization in the CLR, at least explicit finalization. The lack of this support is a huge limitation. To fill this hole they invented this pattern around an interface (IDisposable) on top of the CLR. The programmer has the burden to implement this pattern in order to allow his/her class to be explicitly "finalized" (not finalized from the CLR point of view). Note, for me, IDisposable.Dispose and Object.Finalize are, for all practical matters, synonymous.

I know that efficient deterministic finalization in GC is a classical problem (i.e. finalize the object just after a thread leaves the scope of its last reference), but explicit finalization like the delete operator might be easier to implement.

So perhaps there is not much we can do from C# but workarounds like IDisposable and syntax sugar. It doesn't seem possible to implement the delete operator I proposed with the current CLR.
May 13, 2014 at 11:54 PM
drowa wrote:
I know that efficient deterministic finalization in GC is a classical problem (i.e. finalize the object just after a thread leaves the scope of its last reference), but explicit finalization like the delete operator might be easier to implement.
I don't understand, how is explicit cleanup using delete any better than explicit cleanup using Dispose()? If it's about resetting all the existing references to null, then than pretty much just trades ObjectDisposedException for NullReferenceException.
May 14, 2014 at 12:24 AM
Edited May 14, 2014 at 3:06 AM
You would have everything that matters from the dispose pattern in a much clearer and direct way. You would have a single point for finalization.
May 14, 2014 at 3:45 AM
drowa wrote:
You would have everything that matters from the dispose pattern in a much clearer and direct way. You would have a single point for finalization.
There is already a single point for cleanup: Dispose. The vast majority of programs should not contain a override of Finalize for cleanup purposes (overrides might occasionally be useful to log the fact that an object which should have been cleaned up, wasn't), and most programmers would probably be best served if they had no idea that Finalize even existed. The designers of Java thought Finalize could work as a primary cleanup method, and .NET apparently shared in that belief, but there are many kinds of resources for which Finalize is just plain useless given that it runs in an unspecified threading context, cannot safely acquire locks, has no specified timing relationships with any other object's Finalize method, etc.
May 14, 2014 at 4:48 AM
supercat wrote:
The designers of Java thought Finalize could work as a primary cleanup method, and .NET apparently shared in that belief, but there are many kinds of resources for which Finalize is just plain useless given that it runs in an unspecified threading context, cannot safely acquire locks, has no specified timing relationships with any other object's Finalize method, etc.
Perhaps we should have a more normal running environment inside Finalize. But what happens if you don't call Dispose? Shouldn't we expect Finalize to clean up the object as much as Dispose does?

It seems Eric Lippert expects using (and consequently Dispose) to be used out of politeness, not necessity.

By design, Dispose is for explicit finalization. GC doesn't call Dispose, it calls Finalize instead.
May 14, 2014 at 9:40 AM
Edited May 18, 2014 at 10:38 PM
Again, for me, delete should be part of the CLR and IDisposable shouldn't exist. However, I think I figured out a way to implement something similar with syntactic sugar. It is a compromise where IDisposable would still exist. I think in this case the operator should be called dispose instead of delete.

This syntactic sugar is strictly based on the Dispose Pattern.

The compiler translates:
dispose <expr>;
To:
using (<expr>); // lazy compiler
                // We wouldn't do that on a real implementation. It would be the
                // contrary; 'using' would be implemented using 'dispose'.
And it translates:
public class ResourceHolder : <basetype_interfaces> // can be empty
{
    // Note you should see the two fields ('buffer' and 'resource') and the method
    // 'Close' as optional. In this way, I can represent the two types of recourse
    // holder (basic and complex) with one single example.

    IntPtr buffer;       // unmanaged memory buffer
    SafeHandle resource; // disposable handle to a resource
        
    public ResourceHolder()
    {
        buffer = ...   // allocates memory
        resource = ... // allocates the resource
    }

    public void Close()
    {
        dispose this;
    }

    // This is the method that triggers the compiler to implement the Dispose
    // Pattern under the hood.
    ~ResourceHolder(bool disposing)
    {
       ReleaseBuffer(buffer); // release unmanaged memory
        
        if (disposing)
        {
            // release other disposable objects
            dispose resource;
        }
    }
}
To:
// We only include 'IDisposable' if we don't find it from <basetype_interfaces>
// way up to the root ('Object').
public class ResourceHolder : <basetype_interfaces>, IDisposable
{
    IntPtr buffer;       // unmanaged memory buffer
    SafeHandle resource; // disposable handle to a resource

    public ResourceHolder()
    {
        buffer = ...   // allocates memory
        resource = ... // allocates the resource
    }

    public void Close()
    {
        dispose this;
    }

    // if we derived from 'IDisposable' during translation
    protected virtual void Dispose(bool disposing)
    // if we didn't derive from 'IDisposable' during translation
    protected override void Dispose(bool disposing)
    {
        InnerDispose(disposing);
        // if we didn't derive from 'IDisposable' during translation
        base.Dispose(disposing);
    }

    // compiler choose a name not used by the class ('InnerDispose')
    // basically renamed '~ResourceHolder(bool)' to 'InnerDispose(bool)'
    void InnerDispose(bool disposing) 
    {
        ReleaseBuffer(buffer); // release unmanaged memory
       
        if (disposing)
        {
            // release other disposable objects
            dispose resource;
        }
    }

    // This translation only occurs if none of the base classes way up to
    // 'IDisposable' didn't implemented it already, and if the compiler can't
    // detect InnerDispose(false) <=> NOP (*).
    ~ResourceHolder()
    {
        Dispose(false);
    }
     
    // if we derived from 'IDisposable' during translation
    void IDisposable.Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}
So, using this syntactic sugar, the Dispose Pattern is implemented/hidden for/from the author.

Note some adaptions would be made for sealed classes.

(*) This is the clever part of the syntactic sugar. For example, imagine the above class without the buffer field, in this case the compiler would NOT implement the destructor (Finalize), because, through code analyses, it would detect
InnerDispose(false) been equivalent to NOP (i.e. it doesn't cause side effects).
May 14, 2014 at 1:30 PM
drowa wrote:
However, I think I figured out a way to implement something similar with syntactic sugar.
Except your example goes very much against this:

supercat wrote:
First of all, the only objects should use Finalize for cleanup are generally those whose whole reason for being centers around finalization cleanup. An object of any significant complexity which would hold something that is supposed to be cleaned up when abandoned should should generally, rather than implementing Finalize itself, hold a reference to a finalizable object encapsulating the thing needing cleanup.
So, you're trying to make bad code easier to write, I don't think that's a good idea.
So, using this syntactic sugar you wouldn't need to worry about the dispose pattern and you would never see IDisposable in your code.
You still somehow need to know which objects need to be disposed and I think that finding out whether an object implements IDisposable is much easier than finding whether it overrides Finalize.

And I think the small amount of boilerplate code for the rare cases where you do need to override Finalize isn't worth introducing new syntax.
May 14, 2014 at 3:24 PM
Edited May 14, 2014 at 4:37 PM
svick wrote:
So, you're trying to make bad code easier to write, I don't think that's a good idea.
I'm just trying to follow the pattern recommended by Microsoft in here. Note my example was based on theirs.
And I think the small amount of boilerplate code for the rare cases where you do need to override Finalize isn't worth introducing new syntax.
You got it wrong. It is not only for rare cases. It implements the pattern, as recommended, for all cases.

It seems you didn't pay enough attention to what the compiler is doing.
May 14, 2014 at 4:37 PM
drowa wrote:
It implements the pattern, as recommended, for all cases when using IDisposable.
Your code is based on the pattern for finalizable types, which the page explicitly says should be avoided. And the Basic Dispose Pattern exists to make sure inheritors of the class that override Finalize work correctly (and without code duplication). But if you know your inheritors won't override Finalize (which I think is by far the most common situation), there is no need to do that.
May 14, 2014 at 5:05 PM
Edited May 14, 2014 at 6:28 PM
svick wrote:
Your code is based on the pattern for finalizable types...
You're wrong. It is not only for finalizable types. It is based on the pattern for all types.

Try to see the fields in my code (buffer and resource) as optional.

For example, if you remove buffer from the class then, after the translation, you won't end up with a finalizable type. In this case, the compiler won't override Finalize (i.e. won't implement ~ComplexResourceHolder()) because InnerDispose(false) is equivalent to no op.

Perhaps my code is too concise.
May 14, 2014 at 5:43 PM
svick wrote:
supercat wrote:
First of all, the only objects should use Finalize for cleanup are generally those whose whole reason for being centers around finalization cleanup. An object of any significant complexity which would hold something that is supposed to be cleaned up when abandoned should should generally, rather than implementing Finalize itself, hold a reference to a finalizable object encapsulating the thing needing cleanup.
So, you're trying to make bad code easier to write, I don't think that's a good idea.
What do you mean? If I am designing a type which should wrap an unmanaged connection to some kind of USB device so that code can talk to it using a common interface which includes things like notification events, the correct design is not to have the big wrapper class implement Finalize, but rather to have my big wrapper class hold a reference to a small class object (e.g. a SafeHandle) whose sole purpose is to manage the USB connection and ensure its cleanup. How is that "making bad code easier to write". If anything, I could argue that finalizers exist to facilitate writing bad code, since good code should almost never need them (there are a few situations where languages make it impossible to avoid leaks without horrible nasty code using threadstatic variables, and thus leaky "bad code" is sometimes justifiable, but that doesn't make the leaky code "good").

Some of this discussion reminds me of a conversation between George Lucas and Stephen Spielberg. One really disliked the aliens in the movie "Indiana Jones and the Crystal Skull"; the other called him one day: I've decided the movie isn't going to have aliens "Oh?" Yeah, they're interdimensional beings. "Okay. What do they look like?" Aliens.

My point is that from a language standpoint, it makes sense to distinguish things which are similar, if there's a realistic possibility that in at least some cases the language should treat them differently. There are in fact quite a few cases where I think C# and VB.NET could benefit from adding new usage-specific syntax for some operations which presently share a syntax [e.g. testing non-overloadable reference-equivalence versus testing statically-overloadable value equality]. I can't think of anything C# or VB.NET should do differently from a scope-guard versus a resource. I can imagine some using features which would be helpful for scope guards, which using doesn't presently provide for resources, but those same features would also be useful when guarding resources.

Perhaps it would be helpful if those favor having separate interfaces associated with scope guards could clarify how exactly their behavior should differ from interfaces associated with other resources [note: I would like two new interfaces associated with resources:
interface IDisposableExOnly { void Dispose(Exception pendingException); }
interface IDisposableEx: IDisposable, IDisposableExOnly {  }
Although implementation of IDisposableExOnly without IDispose would tend to be more appropriate for scope guards than for resources, I would think a language shouldn't care about whether something is a resource or a scope guard or other resource, but rather whether it implements IDisposable, IDisposableExOnly, or both (typically via IDisposableEx).
May 14, 2014 at 6:08 PM
drowa wrote:
It seems Eric Lippert expects using (and consequently Dispose) to be used out of politeness, not necessity.
There are many kinds of objects which behave usefully if the consumer calls Dispose when required by contract, and cannot be made to function reasonably if the consumer does not. Demanding that all objects should be safely to abandonable even without calling Dispose is equivalent to stating that July and August cannot be too hot. King Arthur might have been able to do it in Camelot, but in many cases attempting to achieve such a goal would impose substantial costs in both complexity and runtime performance, and in a fair number of cases that objective could not be met at any price.

As a simple example, notification subscriptions--even when processed entirely in managed code--are an "unmanaged resource", in that the GC has no idea how to clean up a short-lived object's subscription to a long-lived object's notifications. Since most notification patterns are not required to provide a thread-safe means of unsubscription, there is no way an object which was notified via Finalize() that it had been abandoned, could safely cancel any subscriptions which it had taken. If an unbounded number of short-lived objects ask a long-lived object to notify them when something happens, and are abandoned without that something ever actually happening, none of the abandoned objects will ever be in a threading context where they can ask that their subscription be canceled.

As another simple example, if an iterator performs a yield return within a lock, and if a client calls GetEnumerator, reads the data from that yield return, and abandons the enumerator without calling Dispose, there is no mechanism via which that lock could ever be released. Even if the iterator had a Finalize method and knew that it held the lock, monitor locks are thread-based rather than token-based, there would be no means by which a method running on the finalizer thread could claim ownership of the lock and release it.

If a particular class wishes to specify that it may safely be created and abandoned even though it holds resources, that contract would overrule the general object contract which says that all IDisposable objects whose individual-class contracts don't specify otherwise may be safely abandoned--and only after--calling Dispose. For some objects like System.Drawing.Font, such behavior is almost essential because other contracts are very murky about ownership. Some other objects like Thread hold unmanaged resources but rely entirely on Finalize and don't even offer a Dispose. In general, though, code which accepts responsibility for an IDisposable object and doesn't either transfer the responsibility or call Dispose is broken.
May 14, 2014 at 6:27 PM
Edited May 14, 2014 at 6:57 PM
Guys, I think the discussion scope-guard versus resource is out of the scope of this thread. Both operators I described here are agnostic of those high level concepts. You could use them for both scope-guards and resources in the same way we use IDisposable.
May 14, 2014 at 6:59 PM
drowa wrote:
Guys, I think the discussion scope-guard versus resource is out of the scope of this thread. Both operators I described here are agnostic of those high level concepts.
I think the point of the discussion was lost along the way, partly due to you changing your proposition in the middle of the thread, partly due to other posters going off topic.

It may be useful to actually explain the problem that you're trying to solve instead of describing what the CLR/compiler should do. For example, what exactly is "dispose <expr>;" supposed to do that "using (<expr>);" doesn't do already?
May 14, 2014 at 7:04 PM
@supercat I think you misunderstood what I was trying to say. I wasn't responding to you, I was using your quote as the principle drowa's suggestion goes against. Sorry for the confusion.
May 14, 2014 at 7:13 PM
Edited May 14, 2014 at 9:56 PM
mdanes wrote:
It may be useful to actually explain the problem that you're trying to solve
It should be obvious, if you read the discussion from the beginning.

The Disposal pattern is a workaround for the lack of explicit finalization in the CLR.

The dispose operator together with the ~(bool) destructor, remove the burden to implement the Dispose pattern from the programmer.

You have everything you had before minus an interface (IDisposable interface) and a pattern (Disposal pattern).

Just compare a traditional implementation of the Dispose pattern with the solution proposed here. All the plumbing code is removed.

And both of my propositions apply perfectly to the title of the discussion.
May 14, 2014 at 8:22 PM
drowa wrote:
It should be obvious, if you read the discussion from the beginning.
Well, it isn't. There's nothing that explains the need for a delete/dispose operator in the whole thread. I understand, at least in part, what you're trying to say with "removes the burden to enforce the Dispose pattern" but that doesn't explain the need for the dispose operator by itself. Is it only intended to avoid the need for a null check?

Anyway, your goal seems to be to have compiler generate the Dispose pattern for you. That certainly has merit. But all this talk about IDisposable being a workaround and this having something to do with the CLR tends to muddy the waters. IDisposable isn't a workaround, it has to exist because Dispose is optional.

You may find interesting the second post in this thread. It also deals with automatic Dispose generation.
May 14, 2014 at 8:58 PM
drowa wrote:
The dispose operator (which is not ideal) does the following:
  1. removes the burden to enforce the Dispose pattern from the programmer
I'm not quite clear what you mean by "burden of enforcing the pattern". Do you mean burden of implementing it? The few bits of boilerplate I see being replaced are less burdensome than would be trying to make a class thread-safe when it has non-thread-safe disposal logic baked into it.
  1. establishes a single point for finalization
"You keep using that word. I do not think it means what you think it means."

The word "finalization" refers to the process via which the Framework runs a method called Finalize on class objects which have been observed as having no strong rooted references outside a special list of objects which override Finalize. A Finalize method should only try to clean up a thing to which it holds a reference if none of the following apply:
  1. The thing cannot safely be cleaned up from a finalizer thread context.
  2. The thing has already been cleaned up.
  3. The thing is already scheduled to be cleaned up.
  4. The thing is still in use by someone, even though the object whose Finalize method is running, isn't.
For the majority of classes that implement IDisposable, at least one of the above conditions would apply to every single object owned thereby. Such classes would therefore not be able to do anything useful within the Finalize method if they were to override it, and thus shouldn't override it.
Just compare a traditional implementation of the Dispose pattern with the solution proposed here. All the plumbing code is removed.
The only thing I see your proposal eliminating is the boilerplate IDisposable.Dispose implementation, and I don't really see what the operator has to do with that.

If you were discussing the ability to apply using to a non-public field or a special kind of property, and having the compiler auto-generate cleanup code for those in a fashion similar to what C++/CLI does to fields of IDisposable types which aren't marked with ^, I could see that as being useful, although there would be a number of implementation issues associated with that.
May 14, 2014 at 9:12 PM
Edited May 14, 2014 at 9:22 PM
mdanes wrote:
drowa wrote:
It should be obvious, if you read the discussion from the beginning.
Well, it isn't. There's nothing that explains the need for a delete/dispose operator in the whole thread. I understand, at least in part, what you're trying to say with "removes the burden to enforce the Dispose pattern" but that doesn't explain the need for the dispose operator by itself. Is it only intended to avoid the need for a null check?
I have to admit I'm not very well articulated and I tend to assume what is obvious for me is obvious for everybody, sorry for that. Also, English is not my first language.

One of the functions of the dispose operator is to help to "eliminate" the IDisposable interface from the face of the programmer. Another one is to make clear disposing instances is on the same "level" of creating instances; you "feel" the new operator and the dispose operator "play" together well in the "same" way constructors and destructors do. Also, if the object is null, dispose is no op where with IDisposable.Dispose you would get NullReferenceException. That's why you usually have something like if (obj != null) obj.Dispose();, but with dispose you would have only dispose obj; . Together those small things produce a much clearer and honest code.

Note the translation of dispose <expr>; to using (<expr>); was a lazy attempt to define dispose. It is know you can define using using try/finally. The dispose operator is practically equivalent to the finally part of that definition. If we define dispose first then we can define using using dispose.
Anyway, your goal seems to be to have compiler generate the Dispose pattern for you. That certainly has merit. But all this talk about IDisposable being a workaround and this having something to do with the CLR tends to muddy the waters. IDisposable isn't a workaround, it has to exist because Dispose is optional.
I just think Dispose is a workaround for the lack of the ability to invoke Finalize explicitly. Of course, if we could invoke Finalize explicitly, it wouldn't behave in the same way it behaves now because now it is optimized for usage by the GC.
You may find interesting the second post in this thread. It also deals with automatic Dispose generation.
Thanks for the link. I would love to see an implicit using like proposed there.
May 14, 2014 at 9:16 PM
PS--Even in those rare cases where an object should have a cleanup finalizer, the "standard dispose pattern" is still wrong. Any object which has a finalizer is subject to involuntary resurrection between the time when the finalizer is scheduled to run, and the time the object ceases to exist. Consequently, a properly-written finalizer must in many cases include some tricky code to ensure that no unmanaged resource can get released twice even if Dispose and Finalizer get called simultaneously. Failure to release a resource may leave that particular resource unusable, but code which can be coaxed into double-freeing resources can cause severe system disruption and/or security breaches. If a class isn't prepared to deal with those issues, it shouldn't implement Finalize (nor use a C# "destructor")
May 14, 2014 at 9:28 PM
Edited May 14, 2014 at 9:34 PM
supercat wrote:
If a class isn't prepared to deal with those issues, it shouldn't...
...allocate unmanaged resources.
May 14, 2014 at 9:46 PM
Guys, I got it the environment from where Finalize is executed over is weird and all; it is optimized for usage by the GC. Obviously, if we could invoke Finalize explicitly then we should expect a different environment. Maybe Finalize() would be similar to Dispose(bool) from the Dispose pattern, where you can check the current environment using the disposing parameter.
May 14, 2014 at 9:58 PM
drowa wrote:
supercat wrote:
If a class isn't prepared to deal with those issues, it shouldn't...
...allocate unmanaged resources.
Many kinds of resources cannot safely be cleaned up by any thread other than the one which created them. The only practical way by which such resources can get cleaned up is by having the thread that created them call Dispose at a suitable time.
I just think Dispose is a workaround for the lack of the ability to invoke Finalize explicitly.
It's a workaround for the fact that the Finalize concept, which .NET borrows from Java, is and always has been fundamentally broken. The creators of .NET realized this early enough to provide a workaround in .NET 1.0, unlike Java which waited until version 7. There are a few cases where Finalize can be useful despite being broken, but in most cases any effort one could spend trying to make a class implement Finalize safely could be better spent ensuring that clients of the class call Dispose when they should.
May 14, 2014 at 10:08 PM
drowa wrote:
Guys, I got it the environment from where Finalize is executed over is weird and all; it is optimized for usage by the GC. Obviously, if we could invoke Finalize explicitly then we should expect a different environment. Maybe Finalize() would be similar to Dispose(bool) from the Dispose pattern, where you can check the current environment using the disposing parameter.
Adding any code to a class's Finalize method will slow down the construction of every instance of that class, even if there would be nothing useful for that method to do when invoked by the GC subsystem. Thus, any class which would be unable to perform cleanup within the finalizer context needs to put its cleanup code somewhere else that other code will be able to find it. IDisposable.Dispose() is the place. Much of the Dispose pattern was written shortly after IDisposable was created, before the limitations of Finalize were understood; it would be a bit simpler if it were being written from scratch today,
May 14, 2014 at 10:09 PM
Edited May 14, 2014 at 10:11 PM
supercat wrote:
Many kinds of resources cannot safely be cleaned up by any thread other than the one which created them. The only practical way by which such resources can get cleaned up is by having the thread that created them call Dispose at a suitable time.
I'm fine with that. You can use the Dispose pattern (and consequently the solution I proposed) for this kind of situation; only clean up the resource if disposing is equal to true. If the user of your class fails to invoke Dispose on the same thread then you have a correctness problem (a leak). I understand sometimes we don't have other option.
May 14, 2014 at 10:09 PM
One of the functions of the dispose operator is to help to "eliminate" the IDisposable interface from the face of the programmer. Another one is to make clear disposing instances is on the same "level" of creating instances; you "feel" the new operator and the dispose operator "play" together well in the "same" way constructors and destructors do.
Well, that's the problem, that this isn't true. This is true in C++ where delete deals with both freeing memory (by itself) and other resources (by calling the destructor). This is not the case in C# because the "freeing memory" part isn't needed. The "other resources" part is needed only in some cases and that's why IDisposable exists, it communicates to the developer of the class that the he has to use delete/dispose/Dispose to get rid of those resources. Without IDisposable you have no choice but to tell the developer to always delete/dispose everything. I don't think that's acceptable in C#.
Also, if the object is null, dispose is no op where with IDisposable.Dispose you would get NullReferenceException. That's why you usually have something like if (obj != null) obj.Dispose();, but with dispose you would have only dispose obj; . Together those small things produce a much clearer and honest code.
Yes, in this regard a delete/dispose operator could be useful. That's exactly what the C++/CLI delete operator does with ref types.
I just think Dispose is a workaround for the lack of the ability to invoke Finalize explicitly.
Nope. For the same reason I mentioned above the ability to invoke Finalize explicitly is problematic, the developer will never know when he has to call Finalize and when not. I think that IDisposable is ok as it is, the real problem is that the language has very limited support for it. It doesn't help in anyway with implementing the Dispose pattern (it has better support for writing finalizers which is funny because those are rarely needed). And the arbitrary introduction of a scope by the using statement is also a bit of a problem as described in the other thread. The language can certainly do better in this regard, C++/CLI showed that this is possible.
May 14, 2014 at 10:18 PM
Let's forget changes on the CLR and on the Finalize. Let's focus on the Dispose pattern and the solution I proposed to eliminate the need to even know it exists.
May 14, 2014 at 10:32 PM
mdanes wrote:
One of the functions of the dispose operator is to help to "eliminate" the IDisposable interface from the face of the programmer. Another one is to make clear disposing instances is on the same "level" of creating instances; you "feel" the new operator and the dispose operator "play" together well in the "same" way constructors and destructors do.
Well, that's the problem, that this isn't true.
You don't have to see the relationship between new and dispose in C# in the same way you see new and delete in C++. Also, memory is a low level concept. I see those operators more in relation to the invocation of constructors and destructors/Dispose.
May 14, 2014 at 11:29 PM
drowa wrote:
Let's forget changes on the CLR and on the Finalize. Let's focus on the Dispose pattern and the solution I proposed to eliminate the need to even know it exists.
The fact that some code exists that implements IDisposable using Microsoft's Dispose Pattern, and other code exists which uses different patterns because of inadequacies in Microsoft's(*) makes it necessary for any types deriving from both styles to understand them.

(*) In some cases, a base-class object that is about to be dismantled may need to do something before any of the derived-class aspects of it become unusable, but the Microsoft Dispose Pattern doesn't allow for that. It's too bad there's no mechanism for a class to both shadow and override a method [wrap the call to derived-class code within a base-class method, instead of vice versa], since that pattern would be useful in many scenarios involving object construction and destruction.
May 14, 2014 at 11:49 PM
Edited May 15, 2014 at 6:01 AM
supercat wrote:
The fact that some code exists that implements IDisposable using Microsoft's Dispose Pattern, and other code exists which uses different patterns because of inadequacies in Microsoft's(*) makes it necessary for any types deriving from both styles to understand them.
You have to pick one or the other. If it is a mix/variation/degeneration then it is not Microsoft's Dispose Pattern.

If you are going to use Microsoft's Dispose Pattern then you don't need to know it because you can just implement the ~<class>(bool) method. In case of a different pattern then you implement it accordingly to its requirements which you should know.

The point is, I don't see collision if this is what you are suggesting.
May 15, 2014 at 8:19 AM
supercat wrote:
The few bits of boilerplate I see being replaced are less burdensome than would be trying to make a class thread-safe when it has non-thread-safe disposal logic baked into it.
I added a thread-safe disposal logic.
May 15, 2014 at 1:09 PM
Edited May 15, 2014 at 1:10 PM
You don't have to see the relationship between new and dispose in C# in the same way you see new and delete in C++.
Exactly. C++'s delete isn't optional, if you used new then you have to use delete at some point, otherwise memory will be leaked. In C# dispose is optional, only some type require Dispose. That's why the IDisposable interface exists, to communicate the requirement to the developer.
I added a thread-safe disposal logic.
Thread safety doesn't belong in there. It doesn't solve any problem and it may lead to deadlocks. Any thread safety measures belong in ~ComplexResourceHolder(bool) because only the author of the type knows how to do this properly.

supercat wrote:
The few bits of boilerplate I see being replaced are less burdensome than would be trying to make a class thread-safe when it has non-thread-safe disposal logic baked into it.
The fact that a class is thread safe doesn't necessarily imply that dispose needs to be thread safe. If you end up with a design where 2 threads are trying to dispose the same object the design is either flawed or simply unusual. I don't think it's valid to keep dismissing a valid proposition based on weird cases that the proposition doesn't solve.
May 15, 2014 at 3:44 PM
drowa wrote:
If you are going to use Microsoft's Dispose Pattern then you don't need to know it because you can just implement the ~<class>(bool) method. In case of a different pattern then you implement it accordingly to its requirements which you should know.
Who is responsible for ensuring that Dispose only gets processed once? If the Dispose implementation handles the check, derived classes' Dispose(bool) methods won't need to. Otherwise every derived class will have to perform the check redundantly.

My normal pattern, BTW, is to have the Dispose implementation include a guard against duplicate calls with a disposalState variable and chains all non-duplicated calls in normal fashion.
The fact that a class is thread safe doesn't necessarily imply that dispose needs to be thread safe.
Ironically, thread-safe Dispose is often most important in classes that are otherwise not thread-safe. With many kinds of low-level I/O, it is possible to abort an operation in progress, but doing so will leave the I/O device in an undefined and useless state. A common paradigm for working with such devices is to have Dispose abort any operation in progress, causing it to throw an exception. For devices which only support blocking I/O, the only way to abort an operation in progress is to call Dispose from another thread (the thread that's running the blocking operation can't do it, since it's blocked!) Some people may consider that an abuse of Dispose, but since aborting the operation will leave the object in a useless state there's often no reason not to have the object immediately perform the cleanup associated with Dispose, and thus no reason to require that Dispose be called after such cleanup is performed.

While I agree that for many kinds of objects, the fact that Dispose is killed while an operation is in progress would indicate a bug, there are other kinds of objects which have well-defined (and useful) semantics for what should happen if Dispose is called while an operation is in progress. If an object would decide to call Dispose on itself (e.g. it finished its appointed task) just as its owner decided its services weren't needed (e.g. because a user hit the "Cancel" button), Dispose could get called twice simultaneously and should be able to deal with that possibility.
May 15, 2014 at 3:59 PM
Edited May 15, 2014 at 3:59 PM
mdanes wrote:
Thread safety doesn't belong in there. It doesn't solve any problem and it may lead to deadlocks. Any thread safety measures belong in ~ComplexResourceHolder(bool) because only the author of the type knows how to do this properly.
I would posit that the lowest-level class which is interested in doing anything with Dispose should use an Interlocked.Exchange on a disposal flag, and chain to Dispose(true) only when appropriate. Unless that same class implements a finalizer/destructor [in which case it should use the same flag], nothing should ever call Dispose(false), and so thread-safety with regard to a finalizer would be irrelevant (ideally, one would also define a new protected virtual sealed Finalize() method so as to prevent any derived classes from overriding Object.Finalize without being regarded as overriding it oneself, but I don't know any way to accomplish that without adding an extra inheritance layer). While Interlocked.Exchange generally imposes some performance cost, having it done once per instance lifetime, regardless of the number of inheritance layers, shouldn't be a burden especially since cache contention on or near the flag should usually be non-existent.

Note that in general, the class which is interested in Dispose would be the same one that implements IDisposable, and so a logical place to put the test would be within the Dispose implementation itself. The one notable exception would be base classes which implement IDisposable not because they need cleanup, but because the same factory methods that return them might also return derived-type objects that do need cleanup. In such case, having the base type implement the logic to avoid redundant Dispose processing might be wasteful, but I'm not sure what better alternative would exist.
May 15, 2014 at 6:13 PM
mdanes wrote:
Thread safety doesn't belong in there. It doesn't solve any problem and it may lead to deadlocks. Any thread safety measures belong in ~ComplexResourceHolder(bool) because only the author of the type knows how to do this properly.
You're right and you're wrong.

I implemented only the minimum necessary. It is a weak thread safety measure which is not expensive.

~ComplexResourceHolder(bool) is thread-safe in a sense that no two threads will invoke it at the same time, and only one thread will invoke it with success (i.e. an exception from this method will allow another thread to invoke it again, but if a thread invoke it without throwing an exception then no other thread will invoke it again). Also, in the case this method is implemented multiple times in a hierarchy of classes, and multiple threads trying to dispose the object at the same time, each implementation of this method can be invoked by a different thread.

So there is still thread safety measures the author could add. For example, nothing prevents ~ComplexResourceHolder(bool) and another method been invoked at the same time.

I argue my measure been necessary because I don't see a way for the author of the class to synchronize ~ComplexResourceHolder(bool) and the disposed field.

I would love to see my measure been attacked. For example, if you think it can lead to a deadlock then show me a scenario where this could happen.
May 15, 2014 at 6:44 PM
supercat wrote:
Who is responsible for ensuring that Dispose only gets processed once?
My thread-safe disposal logic ensures ~ComplexResourceHolder(bool) only gets invoked once.

When you're deriving from a disposable class you need to implement thread safety (if necessary) accordingly to the pattern used by it.
May 15, 2014 at 6:54 PM
Edited May 15, 2014 at 7:04 PM
Just so you know I marked my proposition as answered not to finish the discussion, but just to make clear what the proposition is.
May 15, 2014 at 7:16 PM
drowa wrote:
I would love to see my measure been attacked. For example, if you think it can lead to a deadlock then show me a scenario where this could happen.
The one aspect of thread-safety with which compiler-generated code could usefully concern itself would be ensuring that Dispose(true) only gets called once, since that could be enforced without needing to know anything else about the class. One slight ambiguity would be whether overlapped calls to Dispose should return before the first one completes, and should succeed if the first one fails. Generally the scenarios where overlapped dispose would not represent erroneous behavior are also those where Dispose would not be expected to fail, but code which calls Dispose to stop a process might expect that any resources used by that process will be available by the time Dispose returns.

Another issue is that there are many cases where base-class cleanup would require using virtual methods that would cease to be usable once derived classes perform their cleanup.

One approach to resolve many such issues would be to specify that if a protected bool PreDispose() method is defined, a Dispose implementation should chain to that first, and only call Dispose(true) when PreDispose returns true (note that this would be different from having the base PreDispose() method call Dispose(true), since it would let derived classes perform pre-dispose logic in base-to-derived order (after the base returns true).
May 15, 2014 at 7:25 PM
drowa wrote:
My thread-safe disposal logic ensures ~ComplexResourceHolder(bool) only gets invoked once.
With multiple layers of inheritance, each having its own disposal flag, there will be times when some layers are disposed and others are not. What should a public IsDisposed property report in such cases, and by what means should it know what's going on?
May 15, 2014 at 8:09 PM
drowa wrote:
You're right and you're wrong.
So there is still thread safety measures the author could add. For example, nothing prevents ~ComplexResourceHolder(bool) and another method been invoked at the same time.
Exactly. And because of that it's dubious to have the compiler adding thread safety. There are a few scenarios and this is useful in only one of them:
  1. Dispose doesn't need to be thread safe - either because it simply doesn't need to be or because the code in ~ComplexResourceHolder happens to be thread safe by means of delegating to other thread safe code. For example calling Dispose on a SafeHandle like FileStream and WaitHandle do.
  2. Dispose needs to be thread safe with respect to other methods of the object - this requires custom code in ~ComplexResourceHolder. For example you could use something like ReleaseBuffer(Interlocked.Exchange(ref buffer, IntPtr.Zero)).
  3. All other cases - I can't think of any reasonable example. Let's say that such cases probably exists but they're rare compared to the other 2 cases. This is the only case where ensuring that ~ComplexResourceHolder is called by a single thread at a time is actually useful.
I argue my measure been necessary because I don't see a way for the author of the class to synchronize ~ComplexResourceHolder(bool) and the disposed field.
The "disposed" field is another thing that's not always necessary. The Dispose pattern requires that Dispose can be called multiple times but it doesn't require you to implement this by using a "disposed" field. Like in the thread safety case, such a field may be needed in some cases but there are plenty of cases where there's no need for it. For example FileStream doesn't have it because it relies on SafeHandle's ability to handle multiple Dispose calls.
I would love to see my measure been attacked. For example, if you think it can lead to a deadlock then show me a scenario where this could happen.
The chance that a deadlock can occur during disposal is probably pretty small. But it's something that you always need to consider when you call arbitrary code with a lock held. Especially if the lock is in compiler generated code which people do not see.

supercat wrote:
For devices which only support blocking I/O, the only way to abort an operation in progress is to call Dispose from another thread (the thread that's running the blocking operation can't do it, since it's blocked!) Some people may consider that an abuse of Dispose, but since aborting the operation will leave the object in a useless state there's often no reason not to have the object immediately perform the cleanup associated with Dispose.
So, it's like I said. It's a weird situation that has nothing to do with the subject of this thread.
May 15, 2014 at 9:38 PM
Edited May 15, 2014 at 11:46 PM
supercat wrote:
With multiple layers of inheritance, each having its own disposal flag, there will be times when some layers are disposed and others are not. What should a public IsDisposed property report in such cases, and by what means should it know what's going on?
There is no IsDisposed property in my proposition (neither in the Dispose pattern). The author of the class would need to implement it from scratch, extending the pattern. So this question is a little out of the scope.

However, just for the sake of curiosity, the author could do that on both classes, base and derived:
bool disposed;

~ComplexResourceHolder(bool disposing)
{
    ...
    disposed = true;
}
On base class only:
public virtual bool? IsDisposed
{
    get
    {
        return disposed;
    }
}
And on derived class only:
public override bool? IsDisposed
{
    get
    {
        if (base.IsDisposed.HasValue && base.IsDisposed.Value == disposed)
            return disposed;
        else
            return null;
   }
}
We could try to add thread safety, if necessary.
May 15, 2014 at 11:11 PM
Edited May 16, 2014 at 12:19 AM
mdanes wrote:
drowa wrote:
You're right and you're wrong.
So there is still thread safety measures the author could add. For example, nothing prevents ~ComplexResourceHolder(bool) and another method been invoked at the same time.
Exactly. And because of that it's dubious to have the compiler adding thread safety. There are a few scenarios and this is useful in only one of them:
  1. Dispose doesn't need to be thread safe - either because it simply doesn't need to be or because the code in ~ComplexResourceHolder happens to be thread safe by means of delegating to other thread safe code. For example calling Dispose on a SafeHandle like FileStream and WaitHandle do.
  2. Dispose needs to be thread safe with respect to other methods of the object - this requires custom code in ~ComplexResourceHolder. For example you could use something like ReleaseBuffer(Interlocked.Exchange(ref buffer, IntPtr.Zero)).
  3. All other cases - I can't think of any reasonable example. Let's say that such cases probably exists but they're rare compared to the other 2 cases. This is the only case where ensuring that ~ComplexResourceHolder is called by a single thread at a time is actually useful.
I don't think you would be able to implement item 2 correctly without the measure I added ("weak thread safety of ~(bool)"), assuming you still have "disposed".
The "disposed" field is another thing that's not always necessary.
I don't think we would be able to encapsulate the Dispose pattern into a black box without "disposed" and without the "weak thread safety of ~(bool)". It is a trade off. Those two potentially unnecessary things are harmless, and in case they are not (I can't think of any), you can always implement the Dispose pattern by hand.

If you have to think about the pattern then it is not inside a black box. Look all convenience we are getting for so little.
The chance that a deadlock can occur during disposal is probably pretty small. But it's something that you always need to consider when you call arbitrary code with a lock held. Especially if the lock is in compiler generated code which people do not see.

supercat wrote:
For devices which only support blocking I/O, the only way to abort an operation in progress is to call Dispose from another thread (the thread that's running the blocking operation can't do it, since it's blocked!) Some people may consider that an abuse of Dispose, but since aborting the operation will leave the object in a useless state there's often no reason not to have the object immediately perform the cleanup associated with Dispose.
So, it's like I said. It's a weird situation that has nothing to do with the subject of this thread.
It is weird indeed but note the "weak thread safety of ~(bool)" would allow another thread to call Dispose without problem. So this weird example doesn't challenge the measure.
May 16, 2014 at 12:17 AM
There are many cases where a class will need to implement IDisposable but the Dispose method won't actually do anything. Such classes can properly implement IDisposable.Dispose without having keeping track of whether it has been called previously, and they thus have no reason to use the memory or time necessary to keep track of whether Dispose has been called. The lowest-level that needs to actually do anything in Dispose, however, will likely need to keep track of whether Dispose has been called. I would suggest that it would make sense to have the first class that needs to actually do something in Dispose be responsible for arbitrating the relationship between IDisposable.Dispose and Dispose(true). The first call to IDisposable.Dispose should set in motion a chain of events whereby Dispose(true) will be called at a safe time.

Perhaps the best way to adjust the Dispose Pattern while remaining compatible with existing practices would be to say that classes should either implement IDisposable.Dispose to do nothing, tag it with some attribute, and not implement Dispose(bool) or else should do what's necessary to ensure that making one or more calls to IDisposable.Dispose will cause exactly one call to be made to Dispose(true), and such call will be made at a safe time (possibly after IDisposable.Dispose has returned) and in a safe threading context (e.g. a thread that was busy running a method when IDisposable.Dispose was called).

Adapting the second pattern would mean that a compiler could assume that if the base class implements Dispose(bool), a derived class can attach to it, and if the base class has a tagged do-nothing IDisposable implementation and no other implementation is specified in a derived class, it should implement the standard one. If neither condition applies and no IDisposable implementation is supplied, the compiler should squawk.

How would that sound as an approach?
May 16, 2014 at 6:13 AM
Edited May 16, 2014 at 6:22 AM
supercat wrote:
There are many cases where a class will need to implement IDisposable but the Dispose method won't actually do anything. Such classes can properly implement IDisposable.Dispose without having keeping track of whether it has been called previously, and they thus have no reason to use the memory or time necessary to keep track of whether Dispose has been called.
I'm not sure the memory & time wasted on those cases are significant enough to justify the proposed optimization together with an adjustment of the Dispose pattern.

A much simpler solution is a localized optimization performed by the compiler for cases when the Dispose method doesn't do anything. With the syntactic sugar proposed in this thread, this situation would be equivalent to:
// This class needs to implement 'IDisposable' but the 'Dispose' method doesn't actually do anything.
public class NoResourceHolder : <basetype_interfaces> // can be empty
{
    public NoResourceHolder()
    {
        ...
    }

    // Note it is this destructor that triggers the compiler to implement the Dispose
    // pattern under the hood.
    ~NoResourceHolder(bool disposing)
    {
    }
}
The compiler, after detecting that ~NoResourceHolder(bool) is equivalent to no op independently of the disposing parameter, translates the code to:
// We only include 'IDisposable' if we don't find it from <basetype_interfaces>
// way up to the root ('Object').
public class NoResourceHolder : <basetype_interfaces>, IDisposable
{
    public NoResourceHolder()
    {
        ...
    }

    // if we derived from 'IDisposable' during translation
    protected virtual void Dispose(bool disposing)
    // if we didn't derive from 'IDisposable' during translation
    protected override void Dispose(bool disposing)
    {
        // if we didn't derive from 'IDisposable' during translation
        base.Dispose(disposing);
    }

    // if we derived from 'IDisposable' during translation
    void IDisposable.Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}
Of course, if you are deriving from a disposable class then you don't need to implement the empty destructor ~NoResourceHolder(bool disposing) in the first place, because your class will inherit the implementation of IDisposable, end of story. So let's assume we are not deriving from a disposable class, in this case the compiler produces:
public class NoResourceHolder : <basetype_interfaces>, IDisposable
{
    public NoResourceHolder()
    {
        ...
    }

    protected virtual void Dispose(bool disposing)
    {
    }

    void IDisposable.Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}
May 16, 2014 at 8:24 AM
supercat wrote:
I don't think you would be able to implement item 2 correctly without the measure I added ("weak thread safety of ~(bool)"), assuming you still have "disposed".
As I said, there are many cases where the disposed field isn't necessary.
I don't think we would be able to encapsulate the Dispose pattern into a black box without "disposed" and without the "weak thread safety of ~(bool)". It is a trade off. Those two potentially unnecessary things are harmless, and in case they are not (I can't think of any), you can always implement the Dispose pattern by hand.
They're potentially unnecessary in most cases and as such they aren't harmless. Adding two fields to a class just to cover, say, 25% of the cases isn't a good trade off.
If you have to think about the pattern then it is not inside a black box. Look all convenience we are getting for so little.
At least in case 2 you have to think about it anyway because the weak thread safety provided by the pattern doesn't help. Quite the contrary, it leads to confusion because it will make less knowledgeable people think that the code is safe when in fact it is not. A similar thing happened in the early days with the "synchronized" collections, they were either mostly useless or misunderstood.
It is weird indeed but note the "weak thread safety of ~(bool)" would allow another thread to call Dispose without problem. So this weird example doesn't challenge the measure.
Actually it does because this is likely to fall in case 2. If you close a handle while other threads may be using the object then you better make sure that the closed handle can't be used by other methods in the class. If you don't handle this properly you may introduce a security hole, at least in partial trust scenarios.

I've yet to see a valid example for case 3, where the weak thread safety is actually useful. I found some examples in the framework but it turns out that the code is pretty much broken as is so I don't take them into consideration (see the Component class for an example).
May 16, 2014 at 8:25 AM
Edited May 16, 2014 at 9:11 AM
I think at this point I have a clear idea about my proposition(*), so I feel more comfortable in giving it a name. I will call it Implicit Dispose Pattern in C#.

It is interesting because I started with the Explicit Finalization in CLR and ended up with the Implicit Dispose Pattern in C#. I think it is a good compromise.

(*) dispose operator & ~classname(bool disposing) destructor.
May 16, 2014 at 8:15 PM
Edited May 16, 2014 at 8:36 PM
mdanes wrote:
As I said, there are many cases where the disposed field isn't necessary.
As I said, dispose is harmless in those cases.

mdanes wrote:
They're potentially unnecessary in most cases and as such they aren't harmless. Adding two fields to a class just to cover, say, 25% of the cases isn't a good trade off.
You are underestimating the pros and overestimating the cons. Those two (very small) fields help in allowing the Dispose(bool) method to be called more than once, which is a "DO" from the Microsoft's Disposal Pattern.

mdanes wrote:
At least in case 2 you have to think about it anyway because the weak thread safety provided by the pattern doesn't help.
I don't see why I have to think about the pattern in case 2. The weak thread safety & disposed provided by the compiler does help.

1) It allows disposing be invoked more than once guarantying two things: the uniqueness of a successful dispose attempt and, after that, disposing been no op in a strong-thread-safety way.

2) It guarantees that the execution order of disposing will be serial from bottom to top of the hierarchy of the class in case multiple threads are disposing the same object at the same time.

3) Its doesn't prevent scenarios like the weird example @supercat mentioned. It provides just the right amount of thread-safety.

4) It is so cheap, it doesn't hurt case 1.

They simplify a lot the thread-safety work the author has to do in case 2, and you don't need to know the pattern, at all.

mdanes wrote:
Quite the contrary, it leads to confusion because it will make less knowledgeable people think that the code is safe when in fact it is not. A similar thing happened in the early days with the "synchronized" collections, they were either mostly useless or misunderstood.
Note, case 2 is an advanced case, so an author in this case should know the semantics of the ~classname(bool) destructor, which are reasonable and natural, not confusing.

mdanes wrote:
drowa:
It is weird indeed but note the "weak thread safety of ~(bool)" would allow another thread to call Dispose without problem. So this weird example doesn't challenge the measure.
Actually it does because this is likely to fall in case 2. If you close a handle while other threads may be using the object then you better make sure that the closed handle can't be used by other methods in the class. If you don't handle this properly you may introduce a security hole, at least in partial trust scenarios.
I was trying to say that the example does not break the measure (weak thread safety & disposed). Actually, the example and the measure work pretty well together, see item 3 above.
May 16, 2014 at 9:01 PM
drowa wrote:
You are underestimating the pros and overestimating the cons. Those two (very small) fields help in allowing the Dispose(bool) method to be called more than once, which is a "DO" from the Microsoft's Disposal Pattern.
What pros? I've yet to see any.
I don't see why I have to think about the pattern in case 2. The weak thread safety & disposed provided by the compiler does help.
Nope, it doesn't. That buffer appears to be unmanaged memory and the Dispose intention is to free that memory. Consider what happens if other method in the class is trying to access the unmanaged memory while Dispose does its job. The fact that you seem to have ignored this invalidates the rest of your points.
Note, case 2 is an advanced case, so an author in this case should know the semantics of the ~classname(bool) destructor, which are reasonable and natural, not confusing.
So they said about synchronized collections but when they created the generic collections they no longer used that.
May 16, 2014 at 10:03 PM
Edited May 17, 2014 at 12:12 AM
mdanes
What pros? I've yet to see any.
You must be a plumber.

mdanes
Nope, it doesn't. That buffer appears to be unmanaged memory and the Dispose intention is to free that memory. Consider what happens if other method in the class is trying to access the unmanaged memory while Dispose does its job. The fact that you seem to have ignored this invalidates the rest of your points.
About that example with the buffer, it should be pretty obvious that class was not intended to be thread-safe. Just so you know, that example was based on another example from here.
May 16, 2014 at 11:13 PM
drowa wrote:
Also, about that example with the buffer, it should be pretty obvious that class was not intended to be thread-safe. Just so you know, that example was based on another example from here.
Microsoft's Dispose Pattern documentation vastly understates what is necessary to make a finalizable type safe. Any finalizer which frees an IntPtr without having guarded it with a lock, Interlocked.Exchange, or Interlocked.CompareExchange is wrong. I'm puzzled by why Microsoft would have updated the MDP document to acknowledge the existence of SafeHandle but shows a dangerous finalizer that provides no protection against double-freeing an IntPtr.
May 16, 2014 at 11:45 PM
Edited May 16, 2014 at 11:57 PM
supercat wrote:
I'm puzzled by why Microsoft would have updated the MDP document to acknowledge the existence of SafeHandle but shows a dangerous finalizer that provides no protection against double-freeing an IntPtr.
For me it is clear that the class from the example there was not intended to be thread-safe. Perhaps they propositionally avoided the controversial thread-safety subject in an attempt to prevent distraction from the main subject, the Dispose Pattern. It is not typical to expect a thread to operate over a disposing object. Such atypical scenarios expect experience programmers which already know the Dispose Pattern.

Anyway, I updated my proposition mentioning all thread-safety nuances which I find pertinent.
May 16, 2014 at 11:59 PM
drowa wrote:
For me it is clear that the class from the example there was not intended to be thread-safe.
Finalize is invoked from unknown threading contexts at unknown times. It's not supposed to be called upon an object while it is in use, but even "trusted' code can make that happen very easily. Thus, any class where calling Dispose(false) at unexpected times would cause something to be double-freed should be viewed as being severely defective from a security standpoint.

Note that there's a huge difference between saying legitimate code should not pose a security hazard in such cases versus saying it should "work usefully". If the last surviving reference to an object is in a long WeakReference, and code retrieves it after the GC has decided to that object's finalizer needs to run, it's right and proper that the object may become unexpectedly unusable. What is not right and proper is for such usage to crash the system.
May 17, 2014 at 12:04 AM
It is a controversial subject indeed...
May 17, 2014 at 7:06 AM
drowa wrote:
You must be a plumber.
Really?!
About that example with the buffer, it should be pretty obvious that class was not intended to be thread-safe. Just so you know, that example was based on another example from here.
And the original example doesn't include any thread safety. It should be thread safe if that's a memory buffer but the kind of thread safety that is needed is different from the one that you propose. You need to synchronize the code in ~ComplexResourceHolder with the rest of the code in your class that might access the buffer and by the time you do that the thread safety introduced by the compiler generated code becomes useless. It's as simple as that.
It is a controversial subject indeed...
If you chose too look only at specific, flawed and incomplete examples. If you look at actual implementations you won't find anything controversial about this. Dispose isn't generally required to be thread safe except in case where unmanaged resources are directly involved. And in those cases you're better served by custom code rather than the generic code the compile might produce.

supercat wrote:
Microsoft's Dispose Pattern documentation vastly understates what is necessary to make a finalizable type safe.
Not really. In general, the Microsoft documentation recommends that safe handles are used to control the lifetime of unmanaged resources. The use of safe handles eliminates the need for finalizers and thread safety in Dispose because safe handles already deal with all this stuff. Now, if you choose to ignore the advice and not use safe handles then you're on your own and it's your job to deal with these issues. I see no reason why the compiler would have special support for such a practice, especially given the fact that it can't do it properly.
I'm puzzled by why Microsoft would have updated the MDP document to acknowledge the existence of SafeHandle but shows a dangerous finalizer that provides no protection against double-freeing an IntPtr.
Leftovers from the early days probably. That example should be deleted or updated.
Thus, any class where calling Dispose(false) at unexpected times would cause something to be double-freed should be viewed as being severely defective from a security standpoint.
Exactly. But for memory buffers there's also the issue of "use after free" and that cannot be solved by making only Dispose thread safe.
May 17, 2014 at 7:13 AM
drowa wrote:
supercat wrote:
The few bits of boilerplate I see being replaced are less burdensome than would be trying to make a class thread-safe when it has non-thread-safe disposal logic baked into it.
I added a thread-safe disposal logic.
I changed my mind. I removed back any thread-safety disposal logic. In case you need to synchronize disposing with normal operations, the Dispose Pattern doesn't apply. Also, I noticed my previous solution didn't allow that synchronization.

I also removed disposed since the author can add it, if necessary.

So, congratulations @mdanes, you won.

Thread safety is a controversial subject indeed...
May 17, 2014 at 8:51 AM
@drowa It's not about who won or lost.
May 17, 2014 at 4:32 PM
Edited May 17, 2014 at 4:33 PM
mdanes wrote:
Microsoft's Dispose Pattern documentation vastly understates what is necessary to make a finalizable type safe.
Not really. In general, the Microsoft documentation recommends that safe handles are used to control the lifetime of unmanaged resources. The use of safe handles eliminates the need for finalizers and thread safety in Dispose because safe handles already deal with all this stuff. Now, if you choose to ignore the advice and not use safe handles then you're on your own and it's your job to deal with these issues. I see no reason why the compiler would have special support for such a practice, especially given the fact that it can't do it properly.
If a type encapsulates all its unmanaged resources in safe handles and consequently doesn't need a finalizer, is it a finalizable type?

IMHO, the idea that types which implement IDisposable should have aspirations of supporting Finalize directly (rather than delegating such responsibility to other classes such as SafeHandle derivatives) might have been reasonable in the early days of .NET, but has long since been superceded, and I see no reason to make provisions for it. The standard attachment point for derived-class cleanup is Dispose(bool), but nothing should ever call Dispose(false);. Would you concur?

Also, I'm curious if you have any elegant idea for how a base class should wrap derived-class cleanup in cases where that's necessary. For example, if the purpose of the base class is to receive asynchronous requests and use derived-class logic to respond to them, a proper behavior would be for a Dispose which arrives while an request is being processed (an unavoidable occurrence, since requests can arrive at any time) to stop the acceptance of new requests and call Dispose(true) once processing has completed on all events that were received before the Dispose. Microsoft says that IDisposable.Dispose should be left "as-is", but the only means I see of ensuring that the derived-class objects remain be useful as long as requests are being processed would be to either use different code in IDisposable.Dispose, or else have the base override Dispose(bool) with a sealed method and require that derived classes use some other cleanup approach; of those, I would regard the first as being far less of a deviation from the Microsoft Dispose Pattern. Would you have any alternate suggestions?
May 17, 2014 at 5:38 PM
Edited May 17, 2014 at 8:06 PM
AdamSpeight2008 wrote:
@drowa It's not about who won or lost.
I know. I said that just to illustrate the heated discussion. I was acknowledging he was right and I was wrong.

But I'm very satisfied, btw. When you realize you're wrong, it means you learned something.
May 17, 2014 at 6:25 PM
drowa wrote:
But I'm very satisfied, btw. When you realize you're wrong, it means you learned something.
That's my philosophy in life. Sometimes I get overly argumentative, and may seem prideful, but it's not so much pride as a desire to find out what's right. The most one can learn by merely "agreeing to disagree" is that a contrary view exists; it does not impart any useful information as to whether the contrary view is a result of "cargo cult programming", or whether it is, in fact, correct.

I do not fault people who see Microsoft's Dispose Pattern and think it seems perfect. In the years since the pattern was written, however, a number of shortcomings have become apparent. Because .NET was developed after Java, its designers were able to see some of the problems stemming from Java's lack of any alternative to finalize. and came up with an approach that was better than Java's initial approach, but still didn't resolve all the issues. A lot of .NET is like that, actually: recognizing limitations in Java, and coming up with something that is generally better but doesn't totally overcome the fundamental weaknesses.

It's interesting to note, however, that later versions of Java (I think after .NET 1.0, though I don't know the chronology) included an alternative method of cleanup called a PhantomReference. I've not actually used them, but the basic idea is that the creator of a PhantomReference to an object asks that notification be given when the object ceases to exist, but (unlike Finalize) it does not extend the lifetime of the object. This avoids a difficulty that plagues finalizers, which is that the system can't wait until an object ceases to exist before running its finalizer, and that keeping an object alive for purposes of running a finalizer makes it hard to ensure other code won't resurrect the object and try to use it (whether accidentally or maliciously).

The usage pattern for a PhantomReference would be to have the "public face" of an object which needs to support automatic cleanup encapsulate the guarded resource in another object to only two references exist in the entire universe: in that publicly-exposed object, and in the object which a PhantomReference will notify when the public object is deleted. Once the public object has completely ceased to exist, the notification object can perform the cleanup, secure in the knowledge that no other reference to the guarded resource can possibly exist anywhere in the entire universe.

It's possible to emulate the PhantomReference approach in .NET, but it's a bit awkward. One major annoyance is while a long WeakReference is supposed to remain valid as long as there is any possibility of a strong-rooted reference coming into existence for its target, that only holds as long as a strong rooted reference exists to the WeakReference itself. Were it not for this, it would be possible for the privately-held object to do something like:
private class InternalResource
{
  WeakReference owner;
  protected override void Finalize()
  {
    if (owner.IsAlive) // False alarm--owner got resurrected!
    {
      GC.ReRegisterForFinalize(this);
      return;
    }
    ... cleanup code goes here
  }
  ..
}
As it is, however, the above code wouldn't work because the lack of a rooted strong reference to InternalResource would leave owner subject to unexpected cleanup. To fix that, InternalResource would have to store a reference to owner in some static location, which creates its own interesting challenges.
May 18, 2014 at 9:41 AM
supercat wrote:
If a type encapsulates all its unmanaged resources in safe handles and consequently doesn't need a finalizer, is it a finalizable type?
If a type doesn't override Object.Finalize then the type isn not finalizable.
...The standard attachment point for derived-class cleanup is Dispose(bool), but nothing should ever call Dispose(false);. Would you concur?
In general yes, if you use safe handles then you don't need a finalizer and nothing should call Dispose(false). But some types may do additional work even during finalization. For example, WaitHandle doesn't have a finalizer but FileStream has one because it attempts to flush the write buffer, it seems.

But if this makes you think that you could eliminate the 'disposing' parameter then no, that can't be done. One can inherit from WaitHandle and add a finalizer for who knows what reason so WaitHandle itself has to provide WaitHandle(disposing). This is the main reason the Dispose pattern exist, so disposable class can be properly derived from. If a class is sealed and needs to implement IDisposable then the dispose pattern is NOT needed.
... For example, if the purpose of the base class is to receive asynchronous requests and use derived-class logic to respond to them, a proper behavior would be for a Dispose which arrives while an request is being processed (an unavoidable occurrence, since requests can arrive at any time) to stop the acceptance of new requests and call Dispose(true) once processing has completed on all events that were received before the Dispose. ...
First I'd speculate that such a class design is flawed, the asynchronous receiver should not be a base class and containment rather than inheritance should be used in this case. If you do this then the processor's ~Processor method would simply dispose the receiver. If you must really use inheritance there's nothing stopping you from calling base.Dispose(disposing) from ~Processor. It's really weird but it works because of the requirement that dispose works fine even when called multiple times. I suppose that a better solution would be to have a method like "Stop" on the receiver.
I do not fault people who see Microsoft's Dispose Pattern and think it seems perfect.
But what is imperfect about it? IMO, the main imperfection is that it is a pattern. And it is a pattern because the language does not support it. And that means that the language is imperfect and it's the one who needs fixing. Imagine how C++ would be if there was a "destructor pattern". It would be laughable!
It's interesting to note, however, that later versions of Java (I think after .NET 1.0, though I don't know the chronology) included an alternative method of cleanup called a PhantomReference.
That's a very different issue. The Dispose deals with deterministic cleanup and no matter how you change finalizers you still won't get deterministic finalization. That's simply a consequence of the way GC works. You'd need to drop the current GC and use reference counting. But reference counting has its own issues...

And last but not least:
Microsoft's Dispose Pattern documentation vastly understates what is necessary to make a finalizable type safe. Any finalizer which frees an IntPtr without having guarded it with a lock, Interlocked.Exchange, or Interlocked.CompareExchange is wrong
I mentioned Interlocked in previous post of mine. It turns out that it's wrong too, it's also "weak" thread safety, even for handles (I knew it is no good for memory buffers). Consider a class like FileStream which uses IntPtr to store the file handle. There's a Dispose method which is supposed to call CloseHandle and some other Read method which can read from the file using ReadFile.
  • thread 1 calls the Read method, the current handle value is read and used to call ReadFile
  • somewhere between Read calling ReadFile and the kernel converting the handle to an object pointer thread 1 is suspended because its time slice expired
  • thread 2 calls Dispose, the handle is closed and set to 0. The zeroing part is useless to the existing Read call which already read a non zero the value.
  • at this point thread 1 resumes and uses the old handle value. If you're lucky the handle is invalid and you simply get an error. If you're less lucky the handle was reused and you read from the wrong file - this is a security hole in partial trust scenarios.
So no, there's no way to make only Dispose thread safe. If you need thread safety due to security reasons then the whole class needs to be thread safe. The easiest way is to use a lock object that is acquired by both Dispose and Read so you can't dispose the object while Read is in progress.
May 18, 2014 at 6:11 PM
mdanes wrote:
In general yes, if you use safe handles then you don't need a finalizer and nothing should call Dispose(false). But some types may do additional work even during finalization. For example, WaitHandle doesn't have a finalizer but FileStream has one because it attempts to flush the write buffer, it seems.
I would posit that if a derived type encapsulates aspects that require finalization cleanup but the base class does not implement a finalizer, the proper way to handle that is to have a (possibly nested) finalizable class which encapsulates the things needing cleanup, and have the outer class hold a reference to that. The nested object will have many fewer scenarios to worry about than the main class, and will be in a much better position to deal with them. It will also be far less reliant upon the good behavior of outside code (save only for the fact that if someone holds a reference to the parent object and never lets go, finalization will never occur). The only potential disadvantage I could think of would be that if an object has five separate resources, each of which require finalization cleanup, the GC would have the overhead of managing five finalizable objects rather than one; I would posit that allowing each of those five resources to separately manage its own state, without having to worry about the possibility that e.g. an exception was thrown after the second was constructed, is worth the cost.
But if this makes you think that you could eliminate the 'disposing' parameter then no, that can't be done. One can inherit from WaitHandle and add a finalizer for who knows what reason so WaitHandle itself has to provide WaitHandle(disposing). This is the main reason the Dispose pattern exist, so disposable class can be properly derived from. If a class is sealed and needs to implement IDisposable then the dispose pattern is NOT needed.
The bool parameter is needed, if nothing else, because C# does not allow an interface to be implemented with a protected method, and the designers of the pattern decided to use the name Dispose for the chaining virtual method. There needs to be something to distinguish the outside interface method from the virtual method.

Incidentally, I recall reading a piece which suggested that in a good OOP design classes should not have public virtual methods; instead, they should have public non-virtual methods which chain to protected virtual methods. At first I thought that advice seemed odd, but it allows for the possibility that e.g. it might become necessary to do something before any work is done for a method, and do something else after. While a class contract could e.g. require each implementation to do something like if (GetType() == typeof(this)) makePreparations(); DoWork(); if if (GetType() == typeof(this)) Cleanup(); that's really incredibly ugly. It would be much cleaner to have the base chain to virtAboutToWork();, virtDoWork(); virtDidWork(); in that order. Such code can sometimes be retrofitted if code starts out having a public method DoWork(); which simply chains to virtDoWork(), but such retrofit isn't possible if outside code calls virtual methods directly.
... For example, if the purpose of the base class is to receive asynchronous requests and use derived-class logic to respond to them, a proper behavior would be for a Dispose which arrives while an request is being processed (an unavoidable occurrence, since requests can arrive at any time) to stop the acceptance of new requests and call Dispose(true) once processing has completed on all events that were received before the Dispose. ...
First I'd speculate that such a class design is flawed, the asynchronous receiver should not be a base class and containment rather than inheritance should be used in this case. If you do this then the processor's ~Processor method would simply dispose the receiver. If you must really use inheritance there's nothing stopping you from calling base.Dispose(disposing) from ~Processor. It's really weird but it works because of the requirement that dispose works fine even when called multiple times. I suppose that a better solution would be to have a method like "Stop" on the receiver.
There are many cases where composition is more appropriate than inheritance, but there are others in which the level of interaction between the classes is such that neither side of the relationship can really "contain" the other. Further, plug-in architectures like user-interface designers often expect plug-ins to use a uniform construction method, and aren't really prepared to deal with intricacies of composition.

As for a Stop method, it's called IDisposable.Dispose. Unless an object might allow the ability to restart following a Stop, the only differences I can imagine between the desired behaviors of Stop and Dispose would revolve around how they should handle exceptions, and those could be better handled by having using blocks use an IDisposableEx.Dispose(Exception pendingException) method rather than IDisposable.Dispose() in cases where types implement the former interface.
I do not fault people who see Microsoft's Dispose Pattern and think it seems perfect.
But what is imperfect about it? IMO, the main imperfection is that it is a pattern. And it is a pattern because the language does not support it. And that means that the language is imperfect and it's the one who needs fixing. Imagine how C++ would be if there was a "destructor pattern". It would be laughable!
A proper pattern should allow a base class to wrap the behaviors of its public methods. All that would be necessary for the Dispose pattern to do that would be to acknowledge the legitimacy of implementations where Dispose does something other than Dispose(true); GC.SuppressFinalize(this);, provided only that a call to Dispose will ensure that the virtual Dispose(true) will get called, and a successful return from Dispose(true) calls GC.SuppressFinalize().
It's interesting to note, however, that later versions of Java (I think after .NET 1.0, though I don't know the chronology) included an alternative method of cleanup called a PhantomReference.
That's a very different issue. The Dispose deals with deterministic cleanup and no matter how you change finalizers you still won't get deterministic finalization. That's simply a consequence of the way GC works. You'd need to drop the current GC and use reference counting. But reference counting has its own issues...
Non-deterministic cleanup is different from dispose, true; I think my main point was that the creators of Java recognized that a finalize which keeps an object alive isn't a good cleanup approach; .NET can do much better than chaining to Dispose(false).
And last but not least:
Microsoft's Dispose Pattern documentation vastly understates what is necessary to make a finalizable type safe. Any finalizer which frees an IntPtr without having guarded it with a lock, Interlocked.Exchange, or Interlocked.CompareExchange is wrong
I mentioned Interlocked in previous post of mine. It turns out that it's wrong too, it's also "weak" thread safety, even for handles (I knew it is no good for memory buffers). Consider a class like FileStream which uses IntPtr to store the file handle. There's a Dispose method which is supposed to call CloseHandle and some other Read method which can read from the file using ReadFile.
Code could use Interlocked.Exchange to maintain an invariant that a field would always contain either an invalid handle or a handle which was valid and not in use, slightly more cheaply than it could use locks to uphold the invariant that a lock would only be available when the handle was not in use; I believe that invariant would be sufficient to ensure that improperly-threaded code could not cause a handle-safety violation. Locks might allow cleaner semantics in case of improper multi-thread usage, but if the goal of the class is not to guarantee that improper usage yield nice behavior, but merely to ensure that it won't crash the system, Interlocked methods might offer a cheaper means of achieving that.

In any case, I'll agree that it's not often helpful to make "just" Dispose be thread safe--its guard logic needs to be integrated with other logic for guarding the resource. My intended point wasn't that a compiler-generated thread-safety code which wasn't integrated into anything else would be useful, but rather that I don't see non-thread-safe code as being a proper part of a "general-purpose" implementation. Further, I would think there would be some benefit to having base classes which will need to guard against multiple dispose calls for their own purposes specify, as part of their inheritance contract, that they promise to only allow Dispose(bool) to be called exactly once. Clients that don't wish to rely upon that could add their own redundant field and conditional logic, but I don't really see that it adds value.
May 18, 2014 at 7:33 PM
mdanes wrote:
supercat wrote:
I do not fault people who see Microsoft's Dispose Pattern and think it seems perfect.
But what is imperfect about it? IMO, the main imperfection is that it is a pattern. And it is a pattern because the language does not support it. And that means that the language is imperfect and it's the one who needs fixing. Imagine how C++ would be if there was a "destructor pattern". It would be laughable!
Just to stress that the syntactic sugar proposed would fix this imperfection...

mdanes wrote:
So no, there's no way to make only Dispose thread safe. If you need thread safety due to security reasons then the whole class needs to be thread safe. The easiest way is to use a lock object that is acquired by both Dispose and Read so you can't dispose the object while Read is in progress.
...while still keeping the code compatible with the pattern. This compatibility would allow the syntactic sugar to work with existent code and with extensions of the pattern (like the example above).
May 18, 2014 at 8:10 PM
Edited May 18, 2014 at 10:04 PM
supercat wrote:
My intended point wasn't that a compiler-generated thread-safety code which wasn't integrated into anything else would be useful, but rather that I don't see non-thread-safe code as being a proper part of a "general-purpose" implementation.
Even in concurrent scenarios, Dispose is not usually expected to be called concurrently.

For those cases where Dispose is expected to be called concurrently, you could use an extension of the pattern.
May 18, 2014 at 11:51 PM
Edited May 19, 2014 at 4:03 AM
Since we rarely need to implement finalization (Finalize), specially having SafeHandle in our toolbox, perhaps the below syntactic sugar would be better than the first one.

This syntactic sugar is strictly based on the Dispose Pattern.

The compiler translates:
dispose <expr>;
To:
using (<expr>); // lazy compiler
                // We wouldn't do that on a real implementation. It would be the
                // contrary; 'using' would be implemented using 'dispose'.
And it translates:
public class ResourceHolder : <basetype_interfaces> // can be empty
{
    // Note you should see the following members as optional: 'buffer',
    // 'resource', 'Close' and '~ResourceHolder'. In this way, we can represent
    // the two types of recourse holder (basic and complex) with one single
    // example.

    IntPtr buffer;       // unmanaged memory buffer
    SafeHandle resource; // disposable handle to a resource
        
    public ResourceHolder()
    {
        buffer = ...   // allocates memory
        resource = ... // allocates the resource
    }

    public void Close()
    {
        dispose this;
    }
    
    // This is the method that triggers the compiler to implement the Dispose
    // Pattern under the hood.
    !ResourceHolder()
    {
        // release other disposable objects
        dispose resource;
    }

    // Note only advanced scenarios would require the implementation
    // of this method.
    ~ResourceHolder()
    {
        ReleaseBuffer(buffer); // release unmanaged memory
    }
}
To:
// We only include 'IDisposable' if we don't find it from <basetype_interfaces>
// way up to the root ('Object').
public class ResourceHolder : <basetype_interfaces>, IDisposable
{
    IntPtr buffer;       // unmanaged memory buffer
    SafeHandle resource; // disposable handle to a resource

    public ResourceHolder()
    {
        buffer = ...   // allocates memory
        resource = ... // allocates the resource
    }

    public void Close()
    {
        dispose this;
    }

    // Compiler choose a name not used by the class ('InnerDispose').
    // basically renamed '!ResourceHolder' to 'InnerDispose'
    void InnerDispose() 
    {
        // release other disposable objects
        dispose resource;
    }

    // Obviously, this translation only occurs if '~ResourceHolder'
    // was implemented in the class before translation.
    // Compiler choose a name not used by the class ('InnerFinalize').
    // basically renamed '~ResourceHolder' to 'InnerFinalize'
    void InnerFinalize() 
    {
        ReleaseBuffer(buffer); // release unmanaged memory
    }
    
    // if we derived from 'IDisposable' during translation
    protected virtual void Dispose(bool disposing)
    // if we didn't derive from 'IDisposable' during translation
    protected override void Dispose(bool disposing)
    {
        // if 'InnerFinalize' exists
        InnerFinalize();

        if (disposing)
            InnerDispose();

        // if we didn't derive from 'IDisposable' during translation
        base.Dispose(disposing);
    }

    // This translation only occurs if 'InnerFinalize' exists, and if none of the
    // base classes didn't implemented it already. (*)
    ~ResourceHolder()
    {
        Dispose(false);
    }
     
    // if we derived from 'IDisposable' during translation
    void IDisposable.Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}
So, using this syntactic sugar, the Dispose Pattern is implemented/hidden for/from the author.

Note some adaptions would be made for sealed classes.

(*) This syntactic sugar is not only better for the author, is also better for the compiler, because we eliminated the need for code analysis to infer the need for the final '~ResourceHolder'.
May 19, 2014 at 2:23 AM
Aside from the fact that the C# standard defines a ~className construct and that can't go away, the best thing for a compiler to do with Finalize is nothing. Classes which are going to use Finalize for cleanup should do everything explicitly without relying on any particular pattern, since the exact requirements are highly class-specific.

Beyond that, I find it curious that you omit the one compiler feature I would consider useful with regard to disposal, which would be to have a means of declaring a field to specify that it will be the "owner" any resource identified thereby, and that the compiler should provide some means of automatically calling Dispose on any non-null fields which are thus declared. It should allow user code which wishes to take control over Disposal to control when, as a group, such fields are disposed (e.g. dispose using auto). Having another syntax for calling Dispose on something isn't useful, but having a syntax to declare something and specify that Dispose should be called on it at some point would be.
May 19, 2014 at 3:50 AM
Edited May 19, 2014 at 8:44 AM
supercat wrote:
Aside from the fact that the C# standard defines a ~className construct and that can't go away, the best thing for a compiler to do with Finalize is nothing.
The ~className construct is not going away. The syntax here is just a natural extension of the current standard. Note, if the author remove the !className construct from the class, which trigger this translation, everything would work as defined in the current standard. For example, you would do that when implementing an extension of the Dispose Pattern.

supercat wrote:
Classes which are going to use Finalize for cleanup should do everything explicitly without relying on any particular pattern, since the exact requirements are highly class-specific.
Well, the Dispose Pattern says that finalizables should be part of the pattern, when possible.

supercat wrote:
Having another syntax for calling Dispose on something isn't useful,
I disagree. With this syntax, aside from unusual scenarios, you don't need to know the Dispose Pattern neither the IDisposable interface. Even unexperienced programmers would be able to follow the pattern without effort.

supercat wrote:
but having a syntax to declare something and specify that Dispose should be called on it at some point would be.
In this thread, @bradphelan mentioned a syntax that would do what you suggest. I already though about a translation for that syntax, which would play very nicely with the syntax been proposed here, but the idea is not quite ready yet. Also, a syntax like that probably deserves an exclusive discussion.
May 19, 2014 at 4:29 AM
Edited May 19, 2014 at 6:20 AM
drowa wrote:
Also, a syntax like that probably deserves an exclusive discussion.
Well, after some though, it would be very convenient to extend the syntax here with that syntax. I will post a translation for that soon.
May 19, 2014 at 3:34 PM
drowa wrote:
Well, the Dispose Pattern says that finalizables should be part of the pattern, when possible.
That advice was written before Microsoft discovered that it was was better to encapsulate each unmanaged resource in a separate object (e.g. a SafeHandle--which hadn't been invented when the advice was written).
May 19, 2014 at 4:17 PM
Edited May 19, 2014 at 4:18 PM
supercat wrote:
That advice was written before Microsoft discovered that it was was better to encapsulate each unmanaged resource in a separate object (e.g. a SafeHandle--which hadn't been invented when the advice was written).
If that is true then why does the finalizable example from the Dispose Pattern page, at Microsoft, use a SafeHandle? (rhetorical question)

There, Microsoft advice to avoid writing finalizables, but when you do write one, they advice to bring it to the pattern.
May 19, 2014 at 4:57 PM
drowa wrote:
If that is true then why does the finalizable example from the Dispose Pattern page, at Microsoft, use a SafeHandle? (rhetorical question)
I find it curious that whoever maintains that material updated it to include a SafeHandle in the example, but failed to note that classes whose instances are publicly exposed shouldn't implement cleanup finalizers directly. I think the maintainer wanted to any indication that the previously given advice was should be considered "wrong".

Regardless of what that page says, however, it's not difficult to understand why unmanaged resources should be individually "wrapped" in finalizable objects, rather than having their owner be finalizable. Consider two possible "master" objects, each of which should hold reference to five managed objects and own an unmanaged resource.
  1. Have the master object implement a finalizer, which it uses to clean up the unmanaged resource.
  2. Encapsulate the unmanaged resource in a private finalizable object of its own, and have a non-finalizable master object hold a reference to that (finalizable) resource-holder object.
The second approach is better than the first for a number of reasons. If a master which uses the first approach is abandoned, the GC will not be able to reclaim the master object, nor any of the objects to which it holds references, nor any of the objects to which those objects hold direct or indirect strong references, until after the finalizer for the master object has run. By contrast, if a master which uses the second approach is abandoned, only the small resource-holding object will need to be kept around. Nothing will prevent the reclamation of the master object or any other which is directly or indirectly referenced only by it.

Another advantage of the second board is that is easier to avoid the dangers of premature finalization. If an object that encapsulates resources is finalizable, then it is imperative that all access to those resources be followed by a GC.KeepAlive(this);. Failure to follow that rule may result in the finalizer running while the resources are still in use! If an object is not designed to be finalizable, it's not likely to include GC.KeepAlive() calls. By contrast, if the purpose of an object is to support finalization, such calls will be written in from the start, rather than being an afterthought, Further, since the wrapper class will likely be much smaller than the master class, it will be much easier to inspect its code to ensure that no required GC.Keepalive() calls are missing.

Believe what you will about finalization. All of the .NET programmers and bloggers I've read who understand the intricacies involved recognize that nothing that doesn't need to be in a finalizable classes should be. If you haven't yet done so, take a look at SafeHandle. The whole purpose of that class is to encapsulate one resource which can be identified by an integer, but even just that one "simple" task entails a lot of complexity--far more than can be accommodated with the Microsoft Dispose Pattern. If the MDP isn't adequate for a class whose purpose is simply to guard one handle, what sorts of finalizable class would it be suitable for?
May 19, 2014 at 5:03 PM
drowa wrote:
If that is true then why does the finalizable example from the Dispose Pattern page, at Microsoft, use a SafeHandle? (rhetorical question)
Obviously, because that page was written before SafeHandle was created.

Your latest translation is very tempting because it does away with "disposing". The only concern I have is that ~ResourceHolder() conflicts with the finalizer support we have today in the language. Now, if someone adds both !ResourceHolder() and ~ResourceHolder() it should be clear that ~ResourceHolder has the new meaning. But what if !ResourceHolder() isn't need because there's only a unmanaged resource (IntPtr) to cleanup?

And here we realize that not only that lack of compiler support for the dispose pattern is bad but the fact that compiler support was added for finalizers is bad too. Who exactly needs a finalizer without dispose? Probably no one.

supercat wrote:
A proper pattern should allow a base class to wrap the behaviors of its public methods. All that would be necessary for the Dispose pattern to do that would be to acknowledge the legitimacy of implementations where Dispose does something other than Dispose(true); GC.SuppressFinalize(this);,...
It seems to me that you're trying to make Dispose do more than it was intended to do, clean up resources.
As for a Stop method, it's called IDisposable.Dispose.
Not really. For example, a timer has a Stop method to prevent it from raising events. A control a can be disabled to prevent it from raising input events. A FileStream has Flush so you can force it to write any buffered data without closing it.
... I don't see non-thread-safe code as being a proper part of a "general-purpose" implementation.
And why not? How is Dispose different from, say, List<>.Clear?
May 19, 2014 at 6:17 PM
mdanes wrote:
And here we realize that not only that lack of compiler support for the dispose pattern is bad but the fact that compiler support was added for finalizers is bad too. Who exactly needs a finalizer without dispose? Probably no one.
System.Threading.Thread uses finalizer-based cleanup, though I don't know whether the class itself is finalizable or encapsulates a finalizable object. Within a loop, create a managed thread object, print out its ManagedThreadId, and abandon it. You'll notice that the ID values start being recycled after a GC, but cannot be recycled before.
May 19, 2014 at 7:16 PM
supercat wrote:
A proper pattern should allow a base class to wrap the behaviors of its public methods. All that would be necessary for the Dispose pattern to do that would be to acknowledge the legitimacy of implementations where Dispose does something other than Dispose(true); GC.SuppressFinalize(this);,...
It seems to me that you're trying to make Dispose do more than it was intended to do, clean up resources.
What if such cleanup requires, e.g. acquiring a lock to ensure nothing else is in progress?

mdanes wrote:
And here we realize that not only that lack of compiler support for the dispose pattern is bad but the fact that compiler support was added for finalizers is bad too. Who exactly needs a finalizer without dispose? Probably no one.
System.Threading.Thread uses finalizer-based cleanup, though I don't know whether the class itself is finalizable or encapsulates a finalizable object. Within a loop, create a managed thread object, print out its ManagedThreadId, and abandon it. You'll notice that the ID values start being recycled after a GC, but cannot be recycled before.
As for a Stop method, it's called IDisposable.Dispose.
Not really. For example, ...
In my next sentence, I noted an exception for objects which can suspend service without being rendered permanently useless. Further, note that the whole purpose of IDisposable is that the last "owner" of any object which is no longer wanted--no matter what it is--should have no obligations before abandoning it other than calling IDisposable.Dispose (if implemented).
And why not? How is Dispose different from, say, List<>.Clear?
If something like:
void IDisposable.Dispose()
{
  if (theLocker != null) lock(theLocker) // Note that if `theLocker` hasn't been defined 
  {
    if ((FooState & FooStates.DisposeStarted) == 0)
    {
      FooState |= FooState.DisposeStarted;
      Dispose(true);
      FooState |= FooState.DisposeFinished;
      GC.SuppressFinalize(this);
    }
  }
}
would be considered a legitimate implementation of IDisposable.Dispose() then thread-safety of other code wouldn't be an issue (an implementation like the above could do whatever was necessary to ensure it). If the only legitimate Dispose implementations are those which precisely match the Microsoft Dispose Pattern, then non-thread-safe code in a compiler-generated wrapper could interfere with the use of code that would be necessary to ensure thread-safe disposal.
May 19, 2014 at 8:03 PM
Edited May 19, 2014 at 8:12 PM
mdanes wrote:
Your latest translation is very tempting because it does away with "disposing". The only concern I have is that ~ResourceHolder() conflicts with the finalizer support we have today in the language.
I don't see a conflict happening.
drowa wrote.
The ~className construct is not going away. The syntax here is just a natural extension of the current standard. Note, if the author remove the !className construct from the class, which trigger this translation, everything would work as defined in the current standard. For example, you would do that when implementing an extension of the Dispose Pattern.
.

mdanes wrote:
Now, if someone adds both !ResourceHolder() and ~ResourceHolder() it should be clear that ~ResourceHolder has the new meaning. But what if !ResourceHolder() isn't need because there's only a unmanaged resource (IntPtr) to cleanup?
Note this is an unusual scenario which involves only experienced programmers (remember we have SafeHandle). That being said, in this scenario, all the programmer has to do is to add the following into the class: !ResourceHolder() {}

mdanes wrote:
And here we realize that not only that lack of compiler support for the dispose pattern is bad but the fact that compiler support was added for finalizers is bad too. Who exactly needs a finalizer without dispose? Probably no one.
With the !className construct available in the language, the compiler could add a warning when using ~className without !className. Note you would get the warning even when implementing the Dispose Pattern, or an extension of it, explicitly. Remember that the programmer would be able to suppress the warning by using #pragma warning disable and etcetera. In this way the true intention of the unusual usage of ~className without !className, is double checked.
May 19, 2014 at 9:01 PM
Edited May 19, 2014 at 9:53 PM
supercat wrote:
If the only legitimate Dispose implementations are those which precisely match the Microsoft Dispose Pattern, then non-thread-safe code in a compiler-generated wrapper could interfere with the use of code that would be necessary to ensure thread-safe disposal.
That's not totally true. While not been able to make IDisposable.Dispose atomic, you could still make each !className in the hierarchy atomic.

supercat wrote.
A proper pattern should allow a base class to wrap the behaviors of its public methods. All that would be necessary for the Dispose pattern to do that would be to acknowledge the legitimacy of implementations where Dispose does something other than Dispose(true); GC.SuppressFinalize(this);,...
drowa wrote.
Even in concurrent scenarios, Dispose is not usually expected to be called concurrently.

For those cases where Dispose is expected to be called concurrently, you could use an extension of the pattern.
You are proposing an extension of the Dispose Pattern for concurrent disposing and it introduction into the syntax. At some point I agreed with that.

Before introducing it into the syntax, a consensus about the extended pattern has to be formed. How does it play with the class hierarchy? What scenarios aren't cover by it? Is it worth?

I think the point is, how popular/different extensions of the pattern are.

Perhaps this would deserve its own discussion under General.
May 19, 2014 at 11:56 PM
drowa wrote:
Perhaps this would deserve its own discussion under General.
I created a new thread related to concurrent disposing under APIs instead.
May 20, 2014 at 6:50 AM
supercat wrote:
System.Threading.Thread uses finalizer-based cleanup, though I don't know whether the class itself is finalizable or encapsulates a finalizable object. Within a loop, create a managed thread object, print out its ManagedThreadId, and abandon it. You'll notice that the ID values start being recycled after a GC, but cannot be recycled before.
OK, so finalizer support is still needed in the language. Oh well, let's just hope that since these cases are very rare the overlap between !Foo()+~Foo() and just ~Foo() isn't an issue.
If something like: ... would be considered a legitimate implementation of IDisposable.Dispose() then thread-safety of other code wouldn't be an issue.
The problem with what you are saying is that you still consider that calling Dispose on an object from multiple threads is a sensible thing to do. It's not, plain and simple. And you have not provided one single example where this would actually be needed. Your idea of disposing something from another thread to abort an I/O operation doesn't even work if thread safety is implemented properly.

In the context of Dispose thread safety is needed only to avoid security holes in partial trust scenarios. And even in those scenarios thread safety doesn't mean that it's OK to dispose the object from more than one thread, it is conceivable that Dispose could still miss behave in some way as long as a security hole is not introduced. For example, FileStream uses a SafeHandle and that is thread safe for security reasons. But FileStream's Dispose isn't fully thread safe, it tries to flush the write buffer and there's no thread safety measure for that. And it doesn't need to be because the documentation clearly states that the type isn't thread safe. If you dispose a FileStream from 2 threads and you end up with a corrupted file then that is your problem, not FileStream's problem.

drowa wrote:
Note this is an unusual scenario which involves only experienced programmers (remember we have SafeHandle). That being said, in this scenario, all the programmer has to do is to add the following into the class: !ResourceHolder() {}
OK, so the auto implementation of the dispose pattern by the compiler is triggered solely by !ResourceHolder(). If only ~ResourceHolder() shows up in the class then that's the classic finalizer. Sounds a bit weird and some people my object to this. I'm fine with this because a finalizer without dispose seems to be a rare thing.
May 20, 2014 at 12:06 PM
mdanes wrote:
The problem with what you are saying is that you still consider that calling Dispose on an object from multiple threads is a sensible thing to do. It's not, plain and simple. And you have not provided one single example where this would actually be needed. Your idea of disposing something from another thread to abort an I/O operation doesn't even work if thread safety is implemented properly.
In that scenario, the Dispose method starts by setting a flag and/or using a notification mechanism to signal that a service should shut down, but I didn't want to overcomplicate the example code.
In the context of Dispose thread safety is needed only to avoid security holes in partial trust scenarios. And even in those scenarios thread safety doesn't mean that it's OK to dispose the object from more than one thread, it is conceivable that Dispose could still miss behave in some way as long as a security hole is not introduced. For example, FileStream uses a SafeHandle and that is thread safe for security reasons. But FileStream's Dispose isn't fully thread safe, it tries to flush the write buffer and there's no thread safety measure for that. And it doesn't need to be because the documentation clearly states that the type isn't thread safe. If you dispose a FileStream from 2 threads and you end up with a corrupted file then that is your problem, not FileStream's problem.
True, but if a type's contract specifies that Dispose is supposed to be thread-safe (e.g., a logging class which is designed to accept logging requests from any threading context) there shouldn't be any obstacles to writing a thread-safe Dispose. And I would suggest that the cleanest way of achieving proper semantics is to have IDisposable.Dispose coordinate with the base class before invoking the virtual Dispose. Otherwise the required semantics for each virtual dispose are rather icky.
May 20, 2014 at 12:39 PM
Edited May 20, 2014 at 12:42 PM
mdanes wrote:
OK, so the auto implementation of the dispose pattern by the compiler is triggered solely by !ResourceHolder(). If only ~ResourceHolder() shows up in the class then that's the classic finalizer. Sounds a bit weird and some people my object to this. I'm fine with this because a finalizer without dispose seems to be a rare thing.
Don't forget the warning I proposed.
drowa wrote.
With the !className construct available in the language, the compiler could add a warning when using ~className without !className. Note you would get the warning even when implementing the Dispose Pattern, or an extension of it, explicitly (~className() {Dispose(false);}). Remember that the programmer would be able to suppress the warning by using #pragma warning disable and etcetera. In this way the true intention of the unusual usage of ~className without !className, is double checked.
May 20, 2014 at 4:12 PM
Edited May 21, 2014 at 3:27 PM
I'm improving the semantics of Dispose(bool) from:
// if we derived from 'IDisposable' during translation
protected virtual void Dispose(bool disposing)
// if we didn't derive from 'IDisposable' during translation
protected override void Dispose(bool disposing)
{
    // if 'InnerFinalize' exists
    InnerFinalize();

    if (disposing)
        InnerDispose();

    // if we didn't derive from 'IDisposable' during translation
    base.Dispose(disposing);
}
To:
// if we derived from 'IDisposable' during translation
protected virtual void Dispose(bool disposing)
// if we didn't derive from 'IDisposable' during translation
protected override void Dispose(bool disposing)
{
    if (disposing)
    {
        InnerDispose();

        // if 'InnerFinalize' exists
        InnerFinalize();

        // if we didn't derive from 'IDisposable' during translation
        base.Dispose(disposing);
    }
    else
    {
        try
        {
            // if 'InnerFinalize' exists
            InnerFinalize();
        }
        finally
        {
            // if we didn't derive from 'IDisposable' during translation
            base.Dispose(disposing);
        }
    }
}
The idea is to try to clean up the object as much as possible during finalization while, at the same time, not worrying about the possibility of suppressing exceptions, since they are useless at that point.
May 20, 2014 at 4:16 PM
As a pet project, I forked Roslyn. I will try to implement the highlighted things here. It will be a good opportunity to learn more about Roslyn (and compiler engineering).
May 20, 2014 at 5:44 PM
Edited May 20, 2014 at 5:50 PM
I think I found a construct that will please @supercat while not upsetting @mdanes (you guys represent a polarity between usual and unusual). The construct is straightforward.

The compiler translates:
continue dispose;
to:
this.Dispose(true);
GC.SuppressFinalize(this);
And it translates:
^ResourceHolder()
{
    ...
}
to:
// basically renamed '^ResourceHolder' to 'IDisposable.Dispose'
void IDisposable.Dispose()
{
    ...
}
Like !ResourceHolder construct, the ^ResourceHolder construct also trigger the implementation of the Dispose Pattern, but it can only be used on the first disposable class in the hierarchy.

In case you have ^ResourceHolder without !ResourceHolder then the compiler produces the default !ResourceHolder implicility:
!ResourceHolder()
{
}
In case you have !ResourceHolder without ^ResourceHolder then the compiler produces the default ^ResourceHolder implicility:
^ResourceHolder()
{
    continue dispose;
}
Examples:
class ResourceHolder
{
    readonly object Critical = new object();
    
    public void Read()
    {
        lock (Critical)
        {
            ...
        }
    }

    ^ResourceHolder()
    {
        lock (Critical)
        {
            continue dispose;
        }
    }
}
class ResourceHolder
{
    ^ResourceHolder()
    {
        // Note that if `theLocker` hasn't been defined
        if (theLocker != null) lock(theLocker)
        {
            if ((FooState & FooStates.DisposeStarted) == 0)
            {
                FooState |= FooState.DisposeStarted;
                continue dispose;
                FooState |= FooState.DisposeFinished;
            }
        }
    }
}
class ResourceHolder
{
    int disposed = 0;
    
    ^ResourceHolder()
    {
        if (Interlocked.CompareExchange(ref disposed, 1, 0) == 0)
            continue dispose;
    }
}
May 20, 2014 at 7:25 PM
Note the discussion about extending the Dispose Pattern for concurrent disposing became pointless.

I don't know how to delete it. There is only one post there, and it is mine.

Do you guys know how to delete it?
May 21, 2014 at 6:04 AM
supercat wrote:
In that scenario, the Dispose method starts by setting a flag and/or using a notification mechanism to signal that a service should shut down, but I didn't want to overcomplicate the example code.
And again you're distorting the purpose of Dispose. From "release resources when they're no longer needed" it becomes "I don't know if resources are still needed but I'll send a notification to anyone who might still use the resources that I'm going to release the resources anyway". That's piggybacking functionality on top of the dispose pattern.
True, but if a type's contract specifies that Dispose is supposed to be thread-safe (e.g., a logging class which is designed to accept logging requests from any threading context) there shouldn't be any obstacles to writing a thread-safe Dispose.
And why would the type's contract require that Dispose be thread safe? Sure, the Write method of a Log class usually needs to be thread safe but Dispose does not. You call Dispose on an object because you know that the object is no longer in use. You do not call Dispose to inject a ObjectDisposedExecption or other unwanted behavior into threads that may still be using the object.

drowa wrote:
you guys represent a polarity between usual and unusual
LOL
I think I found a construct that will please @supercat while not upsetting @mdanes
This looks interesting but IMO it's complexity for the sake of complexity
May 21, 2014 at 7:29 PM
Edited May 21, 2014 at 7:41 PM
mdanes wrote:
supercat wrote:
True, but if a type's contract specifies that Dispose is supposed to be thread-safe (e.g., a logging class which is designed to accept logging requests from any threading context) there shouldn't be any obstacles to writing a thread-safe Dispose.
And why would the type's contract require that Dispose be thread safe? Sure, the Write method of a Log class usually needs to be thread safe but Dispose does not. You call Dispose on an object because you know that the object is no longer in use. You do not call Dispose to inject a ObjectDisposedExecption or other unwanted behavior into threads that may still be using the object.
Imagine multiple threads sharing a disposable object where there is not an exclusive thread responsible for disposing it. Each thread will call Dispose on the object when they are done with it. This case is not so unusual. A natural solution is to extend the Dispose Pattern to accommodate reference counting.

mdanes wrote:
drowa wrote:
I think I found a construct that will please @supercat while not upsetting @mdanes
IMO it's complexity for the sake of complexity
On the contrary, it's for the sake of simplicity. A programmer using this construct (^ResourceHolder & continue dispose) will use the rest of the Dispose Pattern implicitly (!ResourceHolder). This simplify not only the work of this programmer, but also the work of any programmer maintaining the code. For example, someone inheriting the code. Or would you prefer to inherit a full explicit implementation of that pattern?
May 22, 2014 at 6:54 AM
drowa wrote:
Imagine multiple threads sharing a disposable object where there is not an exclusive thread responsible for disposing it. Each thread will call Dispose on the object when they are done with it. This case is not so unusual.
Actually it's very unusual because the Dispose contract requires that resources are released when Dispose is called, that implies that the other thread will be toast because it's trying to use a disposed object. Unless all the other thread is doing is calling Dispose, I'd like to hope nobody writes such pointless code.
A natural solution is to extend the Dispose Pattern to accommodate reference counting.
It's not so natural. The Dispose Pattern is simply about the correct implementation of the IDisposable interface and this interface has nothing to do with reference counting. There should be a 'AddRef' like method and the Dispose method's meaning should be changed from "release resources" to "decrement the reference count and release resources if the count reaches 0". You can't add AddRef to the existing IDisposable interface and it's likely that you won't be able to change the meaning of Dispose either, you'll need a separate 'Release' like method. And when you do that you'll find out that it's Release and not Dispose which needs to be thread safe.
On the contrary, it's for the sake of simplicity. A programmer using this construct (^ResourceHolder & continue dispose) will use the rest of the Dispose Pattern implicitly (!ResourceHolder). This simplify not only the work of this programmer, but also the work of any programmer maintaining the code. For example, someone inheriting the code. Or would you prefer to inherit a full explicit implementation of that pattern?
You can't say it's simple when you add new language syntax & semantics, "continue dispose", that doesn't look like anything we've seen before in the language. And it's for the sake of complexity because the scenarios it tries to solve are rare or simply non existent. There have been a lot of "what if" in this discussion but none of that stuff was ever backed by sensible real world use. In some cases it even turned out that the "what if" scenario can't even work properly.

The problem is that we've got this Dispose pattern thing instead of language support. And people look at the pattern and say, ah, but it could do so much more, it could do everything (except the kitchen sink, of course)! If this disposable stuff was built into the language from day one then pretty much nobody would have objected to it for, say, not being thread safe. The same way nobody objects today to the lack of thread safety of the C++'s delete operator (and any other C#/C++ built in operators).
May 22, 2014 at 6:47 PM
Edited May 23, 2014 at 2:38 AM
The following code isn't traditional but IMO it's totally valid:
using System;
using System.IO;
using System.Threading;

class Program
{
    static void Main(string[] args)
    {
        using (
            var @ref = new Reference<StreamWriter>(
                () => new StreamWriter(args[0], true)
            )
        )
        {
            for (int i = 0; i < int.Parse(args[1]); i++)
                new Thread(ThreadStart1).Start(@ref.Add());
        }
    }

    static void ThreadStart1(object data)
    {
        using (var @ref = (Reference<StreamWriter>)data)
        {
            lock (@ref)
            {
                @ref.Value.WriteLine(
                    "ThreadStart: {0}: Thread: {1}",
                    1, Thread.CurrentThread.ManagedThreadId
                );
            }

            new Thread(ThreadStart2).Start(@ref.Add());
        }
    }
    
    static void ThreadStart2(object data)
    {
        using (var @ref = (Reference<StreamWriter>)data)
        {
            lock (@ref)
            {
                @ref.Value.WriteLine(
                    "ThreadStart: {0}: Thread: {1}",
                    2, Thread.CurrentThread.ManagedThreadId
                );
            }
        }
    }
}

sealed class Reference<T> : IDisposable where T : IDisposable
{
    readonly public T Value;

    int Count = 1;

    public Reference(Func<T> value)
    {
        Value = value();
    }

    public Reference<T> Add()
    {
        lock (this)
        {
            Count++;
        }

        return this;
    }

    public void Dispose()
    {
        lock (this)
        {
            if (Count == 1)
            {
                if (Value != null)
                    Value.Dispose();
            }
            else
            {
                Count--;
            }
        }
    }
}
mdanes:
There have been a lot of "what if" in this discussion but none of that stuff was ever backed by sensible real world use. In some cases it even turned out that the "what if" scenario can't even work properly.
There is no need to marginalize those alternative forms of expressing the language. Note the ^ResourceHolder & continue dispose are very straightforward and they don't add any overhead. You don't have to use them. They are there for the colorful.

Using the syntax proposed here, the above Reference class could be coded like the following:
disposable class Reference<T> where T : IDisposable
{
    using public T Value;

    int Count = 1;

    public Reference(Func<T> value)
    {
        Value = value();
    }

    public Reference<T> Add()
    {
        lock (this)
        {
            Count++;
        }

        return this;
    }

    ^Reference()
    {
        lock (this)
        {
            if (Count == 1)
                continue dispose;
            else
                Count--;
        }
    }
}
Note the disposable modifier, maybe this modifier could be the trigger for the implicit Dispose Pattern. I will write more about that (and the using field) later.
May 22, 2014 at 7:03 PM
Edited May 22, 2014 at 11:31 PM
And using implicit using, the above Program class could be coded like the following:
class Program
{
    static void Main(string[] args)
    {
        using var @ref = new Reference<StreamWriter>(
            () => new StreamWriter(args[0], true)
        );
        
        for (int i = 0; i < int.Parse(args[1]); i++)
           new Thread(ThreadStart1).Start(@ref.Add());
    }

    static void ThreadStart1(object data)
    {
        using var @ref = (Reference<StreamWriter>)data;
        
        lock (@ref)
        {
            @ref.Value.WriteLine(
                "ThreadStart: {0}: Thread: {1}",
                1, Thread.CurrentThread.ManagedThreadId
            );
        }

        new Thread(ThreadStart2).Start(@ref.Add());
    }
    
    static void ThreadStart2(object data)
    {
        using var @ref = (Reference<StreamWriter>)data;
        
        lock (@ref)
        {
            @ref.Value.WriteLine(
                "ThreadStart: {0}: Thread: {1}",
                2, Thread.CurrentThread.ManagedThreadId
            );
        }
    }
}
May 22, 2014 at 7:39 PM
mdanes wrote:
And again you're distorting the purpose of Dispose. From "release resources when they're no longer needed" it becomes "I don't know if resources are still needed but I'll send a notification to anyone who might still use the resources that I'm going to release the resources anyway". That's piggybacking functionality on top of the dispose pattern.
Calling IDisposable on a wrapper says that it (the wrapper) is no longer needed. If the wrapper is one of many things which has acquired a shared interest in a resource, then it should release its interest in that resource through whatever means the resource expects. Note that IDisposable is only suitable for unshared objects that hold resources, but that it is entirely proper for an unsharable wrapper to managed shared resources internally using other means when the wrapper are notified that its services are no longer required.
And why would the type's contract require that Dispose be thread safe? Sure, the Write method of a Log class usually needs to be thread safe but Dispose does not. You call Dispose on an object because you know that the object is no longer in use. You do not call Dispose to inject a ObjectDisposedExecption or other unwanted behavior into threads that may still be using the object.
There are many cases where an object will have methods that should be usable from any thread, whose purpose is to provide a notification to anyone who might be interested. The proper behavior for an object which receives a notification in which it has no interest is to simply ignore it.

If a program's UI has a function to enable logging of asynchronous actions to a particular file, or to disable such logging, the proper behavior when the latter option is chosen is to dispose the object. Any events which occur before the Dispose should get logged; those which occur after should not. If an asynchronous event occurs while the Dispose is being processed, it should either cleanly get logged, or cleanly not get logged. It should not be necessary to disturb the processes whose behaviors the users will sometimes want to have logged and sometimes not. For things to work cleanly, though, the base object needs to ensure that it ignores any notifications which it will not be able to process fully.

I've noted before that wrapping calls to protected virtual methods within base-class logic which is invoked via public non-virtual methods is a common pattern. I see no reason to think it is less appropriate with Dispose than in other usage cases. Note that if one doesn't do that, the only way to make locking work correctly is to use recursive locks and have derived classes acquire the base-class lock and hold it while chaining to the base-class method--something which would not be possible if the derived-class wasn't allowed to run code after chaining to the base class.
May 22, 2014 at 11:20 PM
Edited May 22, 2014 at 11:44 PM
For the sake of completeness, from the Program class above, where we have:
new Thread(ThreadStart).Start(@ref.Add());
We should have instead:
var tmp = @ref.Add();

try
{
    new Thread(ThreadStart).Start(tmp);
}
catch
{
    if (tmp != null) tmp.Dispose();
    throw;
}
Or with the syntax here:
var tmp = @ref.Add();

try
{
    new Thread(ThreadStart).Start(tmp);
}
catch
{
    dispose tmp;
    throw;
}
May 23, 2014 at 5:54 AM
supercat wrote:
Calling IDisposable on a wrapper says that it (the wrapper) is no longer needed. If the wrapper is one of many things which has acquired a shared interest in a resource, then it should release its interest in that resource through whatever means the resource expects. Note that IDisposable is only suitable for unshared objects that hold resources, but that it is entirely proper for an unsharable wrapper to managed shared resources internally using other means when the wrapper are notified that its services are no longer required.
That's all true but that doesn't justify in any way the example you posted earlier. All this stuff you're describing now can be summarized to something like
!Wrapper() { resource.Release(); }
There are many cases where an object will have methods that should be usable from any thread, whose purpose is to provide a notification to anyone who might be interested. The proper behavior for an object which receives a notification in which it has no interest is to simply ignore it.

If a program's UI has a function to enable logging of asynchronous actions to a particular file, or to disable such logging, the proper behavior when the latter option is chosen is to dispose the object. Any events which occur before the Dispose should get logged; those which occur after should not. If an asynchronous event occurs while the Dispose is being processed, it should either cleanly get logged, or cleanly not get logged. It should not be necessary to disturb the processes whose behaviors the users will sometimes want to have logged and sometimes not. For things to work cleanly, though, the base object needs to ensure that it ignores any notifications which it will not be able to process fully.
It seems to me that you have repurposed Dispose to do things that it wasn't intended to do. "Disable logging" isn't quite the same thing as "close log file", the later implies the former but not the other way around. There are more natural way to implement this kind of stuff, ways that do not abuse the IDisposable interface and the dispose pattern.

drowa wrote:
The following code isn't traditional but IMO it's totally valid:
Only if you choose to ignore the documented IDisposable contract.

@drowa & @supercat: This is no longer a debate about a new language feature. This is a competition to find news ways to abuse the IDisposable interface. Some of those ways are simply bad/dubious design. Some have some merit but it will probably be difficult to convince the language team to implement a language feature in a particular way just because some people are willing to bend the rules. In any case, I'm done here because I'm just wasting my time.
May 23, 2014 at 12:52 PM
I think the middle portion of this thread represented a reasonable feature. Before that it was unclear, and after that it seemed to get derailed by missing the fact that the new syntax would be an addition to, not a replacement of, the existing ability to use the IDisposable interface.

dispose operator

The new dispose operator would have the following form, where x is an expression:
dispose x;
The semantics of the previous statement would be equivalent to the following:
using (x) { }

Disposers

A disposer is a member that implements the actions required to release the managed resources owned by an instance of the class. A disposer is declared using a disposer-declaration:
disposer-declaration:
! identifier ( ) disposer-body

disposer-body:
block
The identifier of a disposer-declaration must name the class in which the disposer is declared. If any other name is specified, a compile-time error occurs.

The inclusion of a disposer would trigger the following at compile time.
  1. The enclosing type implements IDisposable.
    1. If a base class of the enclosing type implements IDisposable, no alterations are made for this step.
    2. If no base class of the enclosing type implements IDisposable, and the enclosing type does not list IDisposable in the interface-type-list, then IDisposable is added to the types interface-type-list, and the following method is added to the class.
      void IDisposable.Dispose()
      {
          Dispose(true);
          GC.SuppressFinalize(this);
      }
      
    3. If no base class of the enclosing type implements IDisposable, and the enclosing type already lists IDisposable in the interface-type-list, then the following method is added to the class.
      public void Dispose()
      {
          Dispose(true);
          GC.SuppressFinalize(this);
      }
      
  2. It is a compile-time error for a class to contain both a disposer-declaration and a method with a signature that aliases either of the forms potentially added by the previous rule.
  3. A protected method Dispose(bool) is added to the class. It is a compile-time error for a type to contain a disposer-declaration when the following rules lead to an incorrect program.
    1. If a base class of the enclosing type implements IDisposable, the signature of the added method is the following:
      protected override void Dispose(bool)
      
    2. If no base class of the enclosing type implements IDisposable, the signature of the added method is the following:
      protected virtual void Dispose(bool)
      
  4. The contents of the Dispose(bool) method are semantically equivalent to the following:
    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            [the disposer-body]
        }
    
        base.Dispose(disposing);
    }
    

Relation between disposers and finalizers

When a class contains both a disposer-declaration and a destructor-declaration, the semantics of the destructor-declaration are altered.
If the altered semantics of the destructor-declaration are not desired for a type, users may manually use the existing IDisposable interface rather than declare a disposer in the class.
  1. It is a compile-time error for a type to contain both a disposer-declaration and an external destructor.
  2. The Dispose(bool) method created during translation of the disposer is changed to the following.
    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            [the disposer-body]
        }
    
        [the destructor-body]
        base.Dispose(disposing);
    }
    
  3. The destructor-body for the destructor is replaced with the following after its original contents are moved as described in the previous step.
    {
        Dispose(false);
    }
    

Implementation considerations

The placeholders [the disposer-body] and [the destructor-body] which appear in the previous code may be implemented either in-line or through the introduction of generated private instance methods in the enclosing type, provided the the semantics of each of these blocks is fully preserved. If generated helper methods are used, the names of these methods are unspecified with the requirement that the method not alias the name of any other member of the class.
May 23, 2014 at 8:40 PM
Edited May 23, 2014 at 11:33 PM
Thanks, @sharwell, for adding some formalism into the discussion.

sharwell wrote:
I think the middle portion of this thread represented a reasonable feature. Before that it was unclear, and after that it seemed to get derailed by missing the fact that the new syntax would be an addition to, not a replacement of, the existing ability to use the IDisposable interface.
That fact is true. But note what I propose would deliberately discourage any attempt to explicitly implement the IDisposable interface (e.g. compiler warnings). The idea is to bring the Dispose Pattern from the API into the language; the Dispose Pattern today is an elephant in the room. Btw, I see the IDisposable interface as an inseparable part of the Dispose Pattern (i.e. I can't talk about the IDisposable interface without talking about the Dispose Pattern).

The ^ResourceHolder & continue dispose construct would normalize customization of the pattern. With this construct available, I would feel OK in getting warnings when, for whatever reason, explicitly implementing the IDisposable interface.

I don't agree with your item 1.3 because, with the dispose operator available, there is no need to call the Dispose method directly. Accordingly with what I propose, calling the Dispose method directly would be considered deprecated, generating a warning (e.g. "consider using the 'dispose' operator instead"). Hidden the IDisposable interface (as much as we can) is part of the idea in bringing the Dispose Pattern into the language.

After some though, I think we should have a modifier which would work in conjunction with disposers
(!ResourceHolder).

The disposable modifier could be applied to classes, interfaces and, even to, structs.

For interfaces and when <interface-list> does not contain IDisposable directly, the compiler translates:
disposable interface IResourceHolder : <interface-list> // can be empty
{
    ...
}
to:
interface IResourceHolder : <interface-list>, IDisposable
{
    ...
}
For classes and when <interface-list> does not contain IDisposable directly, the compiler translates:
disposable class ResourceHolder : <type>, <interface-list> // both can be empty
{
    ...
}
to:
class ResourceHolder : <type>, <interface-list>, IDisposable
{
    ...
}
If <interface-list> contains IDisposable directly, then the compiler displays an error like the following: "'System.IDisposable' is listed in interface list conflicting with the 'disposable' modifier. You should choose one or the other."

If <interface-list> contains IDisposable directly and the disposable modifier is missing, then the compiler displays a warning like the following: "Explicitly deriving from 'System.IDisposable' is deprecated, consider using the 'disposable' modifier instead."

Implementing a disposer explicitly, or implicitly (e. g. through ^ResourceHolder or through using <field>), is only allowed on disposable classes/structs. Note, it does not matter if the disposable type used the disposable modifier, or if it used the IDisposable interface directly, or if didn't use any of that (i.e. it derived from a disposable class/interface thus it's disposable automatically). In other words, no matter how the type became disposable, it is allowed to implement a disposer explicitly/implicitly.
May 23, 2014 at 9:11 PM
Edited May 23, 2014 at 10:36 PM
Note what I proposed here doesn't conflict to your proposition about asynchronous disposables. You would be able to implement asynchronous disposables without getting warnings.

The System.IAsyncDisposable interface
public disposable interface IAsyncDisposable
{
    Task DisposeAsync();
}
Also, from your proposition and from the definition of the dispose operator, you would be able to use the operator with asynchronous disposables.
May 24, 2014 at 2:19 AM
My primary concern centers around what appears to be a solution that is at least as complex as what users are required to do currently. Rather than introduce syntactic sugar for the most common case, you are introducing new syntax that exposes the same set of semantic features already provided by the language. This requires users to still have a full understanding of resource management (few do) while also learning new syntax.

If you look at popular constructs which are defined as compile-time expansions (such as using), you'll find that the goal is providing a simple and intuitive syntax for the majority of cases, but the alternative implementation is certainly not deprecated (and occasionally required when the semantics of using are not a fit for a particular use case).

The proposal I offered has the following benefits:
  1. It is optimized for the primary use cases which appear in C# code, while avoiding complexity that only solves edge case scenarios that are accessible by other means.
  2. It is substantially simpler than the proposal you are leaning towards, which has major practical advantages in both defining and implementing the feature (with complete tests) as well as educating users about the scenarios where it fits well.
  3. It is defined in a manner that encourages interoperability with code that is not written using the new syntax.
Simple and intuitive does not always win over providing features, but when dealing with a case where all of the features are readily available through other means (such as implementing IDisposable.Dispose and Finalize), simple and intuitive is really the only driving factor for this new syntax. All of the suggestions here need to be weighed against this fact.
May 24, 2014 at 8:15 AM
Edited May 24, 2014 at 8:26 PM
Perhaps my proposal would work better with a compiler option.
/implicitdispose[+ | -]
Use this option if you want to use Implicit Dispose Pattern.

If you do not specify /implicitdispose, all the facility for the Implicit Dispose Pattern won't be used by the compiler (same as specifying /implicitdispose-). Specifying /implicitdispose is the same as specifying /implicitdispose+.

If you don't use this option then the compiler behaves exactly as it behaves today. In other words, no dispose operator, no disposers, no disposable modifier, no implicit implementation of the Dispose Pattern and etcetera.

If you do use this option then you will NOT be able to explicitly implement the Dispose pattern. Note this does not discourage interoperability with code that is not using this option, because the resulting assemblies are indistinguishable from each other.

For example, when using this option, the code
obj.Dispose();
would generate the error (not warning):
Do not invoke 'IDisposable.Dispose'. Instead, use the 'dispose' operator.
The code
public interface IAsyncDisposable : IDisposable
{
    Task AsyncDispose();
}
would generate the error:
Do not derive from 'IDisposable'. Instead, use the 'disposable' modifier.
The code
disposable class ResourceHolder
{
    public void Dispose()
    {
        ...
    }
or
disposable class ResourceHolder
{
    void IDisposable.Dispose()
    {
        ...
    }
}
would generate the error:
Do not explicitly implement 'IDisposable'.
The code
disposable class ResourceHolder
{
    protected virtual void Dispose(bool disposing)
    {
        ...
    }
}
would generate the error:
Do not explicitly implement 'Dispose(bool)' on a disposable type.
The code
Dispose(true);
would generate the error:
Do not explicitly invoke 'Dispose(bool)' of a disposable type.
With this compiler option, the semantics of my proposal would be different, mainly in respect of the trigger of the implicit implementation of the Dispose Pattern.

In the following both codes, <type> is NOT disposable.
In the first one, there is at least one disposable interface in <interface-list>
class ResourceHolder : <type>, <interface-list>
{
}
this code is translated to:
class ResourceHolder : <type>, <interface-list>
{
    // disposers (always moved into 'Dispose(bool)')
    !ResourceHolder()
    {
    }

    // dispose entry point (only implemented in the first 
    // disposable of the hierarchy)
    ^ResourceHolder()
    {
        continue dispose;
    }
}
In the second one, there aren't any disposable interface in <interface-list>
disposable class ResourceHolder : <type>, <interface-list>
{
}
this code is translated to:
class ResourceHolder : <type>, <interface-list>, IDisposable
{
    !ResourceHolder()
    {
    }

    ^ResourceHolder()
    {
        continue dispose;
    }
}
Note the only difference is the IDisposable added to the interface list.

Now the following code:
class ResourceHolder
{
    !ResourceHolder()
    {
        ...
    }
}
would generate the error:
Disposers are only allowed on disposable types.
So only when it is necessary and valid for the Dispose Pattern to be implemented, the compiler will implement it.

Another significant difference is that you won't be able to implement true destructors explicitly.
disposable class ResourceHolder
{
    // pseudo-destructors (always moved into 'Dispose(bool)')
    ~ResourceHolder() 
    {                 
        ...
    }
}
If the disposable modifier in the example above is missing:
class ResourceHolder
{
    ~ResourceHolder()
    {
        ...
    }
}
then we would get the error:
Destructors are only allowed on disposable types.
So when using this option only pseudo-destructors exist in the language.

As I said before, pseudo-destructors and dispose entry points would be used only on very unusual situations, while in typical situations, only disposers would be used. So I do think this syntax makes the code much more simple and intuitive. It also allows the addition of the using <field> construct, witch I will explain later, turning the language even more simple and intuitive.

Another point is that we won't be the last generation of C# programmers. Imagine someone learning the language using /implicitdispose+ (/elephant-), and then imagine someone learning the language using
/implicitdispose- (/elephant+).

Note all those examples are just to give an idea on how things would work with the addition of this compiler option. I will try to summarize my entire proposal in a more formal way, like @sharwell did with his.
May 27, 2014 at 4:09 AM
Edited May 27, 2014 at 4:10 AM
Slightly off topic, but are there any plans to reduce the amount of boiler plate code that needs to be written when implementing the Dispose pattern? I often find myself cutting and pasting a lot of code when implementing IDisposable and people often put the same thing in their implementation of Finalize (if they provide one) and Dispose methods, the only thing that seems to differ is the implementation of Dispose(bool) and it usually has the same structure, with a flag protecting access to managed resources and releasing of unmanaged resources outside of the flag.

From MSDN doco:
public class ComplexResourceHolder : IDisposable {
 
    private IntPtr buffer; // unmanaged memory buffer
    private SafeHandle resource; // disposable handle to a resource
        
    public ComplexResourceHolder(){
        this.buffer = ... // allocates memory
        this.resource = ... // allocates the resource
    }

    protected virtual void Dispose(bool disposing){
            ReleaseBuffer(buffer); // release unmanaged memory
        if (disposing){ // release other disposable objects
            if (resource!= null) resource.Dispose();
        }
    }
 
    ~ ComplexResourceHolder(){   <-- boiler plate
        Dispose(false);
    }
 
    public void Dispose(){    <-- boiler plate
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}
Developer
May 27, 2014 at 6:05 AM
NedStpyanov wrote:
Slightly off topic, but are there any plans to reduce the amount of boiler plate code that needs to be written when implementing the Dispose pattern?
There are no plans in this round of language changes related to disposing objects. See HERE for slightly out-of-date notes on the planned language changes and their status.