Should IDisposable objects stored in collections be disposed manually?

Question

I have a stack of WindowsImpersonationContext objects, which are IDisposable type. Now consider these two operations:

Stack<WindowsImpersonationContext> stack = new Stack<WindowsImpersonationContext>();

public void BeginOperation()
{
    // Push object to stack
    var context = WindowsIdentity.Impersonate(...);
    stack.Push(context);
}

public void EndOperation()
{
    // Pop object from stack.
    if (stack.Count != 0)
    {
        var context = stack.Pop();
        ... do various stuff with 'context' ...
        context.Dispose();  // should this be called once the object is not needed anymore?
    }
}

Should I call Dispose() manually if I don't want to wait for the GC to do it? Is it safe to do it like this if I'm 100% no other code will try to use the context object referenced in the stack? To be precise, if I don't explicitly call context.Dispose(), I assume that even after the execution returns from EndOperation(), I'm at the mercy of GC in regard to disposing resources occupied by the 'context' object, despite it not being referenced by the stack anymore? Is my reasoning correct?

Also, using the above code, is it correct to claim that after executing the following set of operations, the stack will end up empty in terms of resources occupied (i.e. all objects it used to contain are disposed at the end, and I don't need to worry about disposing anything elsewhere anymore):

BeginOperation();
  BeginOperation();
  EndOperation();
EndOperation();

EDIT: I realise that using can/should be used for safety purposes, but mainly I'm concerned with what's going on in the background in terms of collections/GC, regardless of the syntax ...

Answer 1

If you don't need it anymore and it implements IDisposable use the using-statement.

public void EndOperation()
{
    // Pop object from stack.
    if (stack.Count != 0)
    {
        using(var context = stack.Pop())
        {
            // ... do various stuff with 'context' ...
        }
    }
}

Apart from that Stack.Pop is implemented in this way(ILSPy, .NET 4):

// System.Collections.Stack
public virtual object Pop()
{
    if (this._size == 0)
    {
        throw new InvalidOperationException(Environment.GetResourceString("InvalidOperation_EmptyStack"));
    }
    this._version++;
    object result = this._array[--this._size];
    this._array[this._size] = null;
    return result;
}

So you see that there's no reference anymore in the stack after Pop.

In general you should use using (or try-finally) whenever you use unmanaged resources. On this way it is ensured that they are disposed even on error.

Answer 2

If you are sure that nothing other than the stack had a reference to the object in question, then yes: You should dispose it yourself manually.

I'd change your code to look more like this:

public void EndOperation()
{
    // Pop object from stack.
    if (stack.Count != 0)
    {
        using (var context = stack.Pop())
        {
            ... do various stuff with 'context' ...
        }
    }
}

That way it will be disposed even if an exception occurs in the "do various stuff" block.

If you don't dispose it yourself, then you rely on a finalizer being implemented for the class being disposed - and even if a finalizer is implemented, it may be called too late (imagine if it was an open file handle).

Answer 3

You can use using statement but before you should implements IDisposable.