Dispose of objects in reverse order of creation?

Question

I face a recurring problem in the code I write: modifying some global value (I'll use a registry value just as an example) and then trying to revert the modifications to original state.

I thought I'd try to use IDisposable to solve this problem. When created, the object would read the registry value, store it locally, and then modify it. When destroyed, it would revert the setting. It would be used something like this:

using(RegistryModification mod = new RegistryModification("HKCR\SomeValue", 42))
{
    // reg value now modified to 42, do stuff
} // Object gets disposed, which contains code to revert back the setting

Should work great, if only 1 modification is ever made. But if multiple modifications are made, or the caller does not create the objects with the 'using' construct, I can see trouble occuring.

public void Foo()
{
    // assume HKCR\SomeValue starts as '13'

    // First object reads HKCR\SomeValue and stores '13', then modifies to 42
    RegistryModification mod1 = new RegistryModification("HKCR\SomeValue", 42); 

    // Next object reads HKCR\SomeValue and stores '42', then modifies to 12
    RegistryModification mod2 = new RegistryModification("HKCR\SomeValue", 12);

}
// objects are destroyed. But if mod1 was destroyed first, followed by mod2,
// the reg value is set to 42 and not 13. Bad!!!

The problem worsens if a caller manually disposes the object. This leads me to think my approach is, simply, flawed.

Is there some sort of accepted pattern to solve this problem? I was thinking adding a static stack to the class might help.

Is the order that objects are destroyed guaranteed in any way? I thought I'd try with IDisposable, but am all ears for other solutions.

Answer 1

IDisposable does not guarante a rollback in this fashion what you are describing is a rollback. The Dispose() method is not meant for that purpose rather it is responsible for the releasing of native resources that the object holds (network connections, files etc)

However a way to revert to state could be done like this

public void Foo(SomeObjectType theObject)
{
    int initialValue = theObject.SomeProperty;
    theObject.SomeProperty = 25;
    Console.Out.WriteLine("Property is:" + theObject.SomeProperty);

    // reset object.
    theObject.SomeProperty = initialValue;
    Console.Out.WriteLine("Property oringinal value is:" + theObject.SomeProperty);
}

Remember just because a resource is disposed of it does not reverse actions that were performed using it, if you dispose of a database connection it does not undo work carried out with it, just destroys the object.

Unless you override Dispose() with rollback code, which is a misuse of that method, then it will not pull out your values, that is your resonsibility as the programmer. The reason I say it is a misuse of the dispose method is because the .net documentation states the following for Dispose()

Use this method to close or release unmanaged resources such as files, streams, and handles held by an instance of the class that implements this interface. By convention, this method is used for all tasks associated with freeing resources held by an object, or preparing an object for reuse.

This generally means for example freeing handles to heavyweight stuff ( say for example some GDI resource). Native resources must be freed or sometimes placed into a certain state to avoid memory leaks or unwanted consequences of access. It is easy to say in this case that perhaps in your Dispose method you should set your registry back to the state it was before, but It is my view this is not the intent of the dispose method. The intent is to release resource and put them back into a state where they can be used again. What you want to do is reset a value (which is essentially another set operation), Doing that work in a custom dispose method means you also shorted reuse opportunities later in different contexts.

What I am saying is that when you have finished modifying you must write explicit code to set the objects back to their initial state. You could probably store this in a data structure (say a stack) and read back the values if there were multiple operations, or just use a simple method like above for one operation.

Answer 2

Sounds like you want to use the System.Collections.Generic.Stack<T> implementation. Each modification would push a value onto the stack, and each "undo" would pop the value off the stack.

http://msdn.microsoft.com/en-us/library/3278tedw.aspx

Answer 3

Use a static factory method instead of a constructor to get a RegistryModification object, and have a static stack of actions (some simplified representation of the RegistryModifcation that the class can understand but doesn't need an entire object to represent, indicating whether its object has been disposed) in the RegistryModification class. When generating a new one, stick a representation on the stack. When disposing, mark the representation as representing a disposed object, and try to reverse the actions in the stack from the top down (stopping when you find an action from an object that is not disposed).

Not sure how much this costs in terms using the memory you're trying to free by disposing, but it should work.

Answer 4

There are two general patterns for implementing the pattern: "Prepare to do something; do it; clean up":

// Pattern #1

void FancyDoSomething(MethodInvoker ThingToDo)
{
  try
  {
    PrepareToDoSomething();
    ThingToDo.Invoke(); // The ".Invoke" is optional; the parens aren't.
  }
  finally
  {
    Cleanup();
  }
}

void myCode(void)
{
  FancyDoSomething( () => {Stuff to do goes here});
}

// Pattern #2:

// Define an ActionWrapper so its constructor prepares to do something
// and its Dispose method does the required cleanup.  Then...

void myCode(void)
{
  using(var wrap = new ActionWrapper())
  {
    Stuff to do goes here
  }
}

Pattern #2 is in some ways more versatile, since it allows for usage patterns that don't follow strict nesting rules. This is part of the reason that IEnumerator<T> uses that pattern instead of simply having an enumeration method that takes a delegate and calling it on every list item. Trying to do something like a list merge if one had to obey nesting semantics with enumerators would be awkward at best.

On the other hand, some types of guarded resources can only be meaningfully used in strictly-nested fashion. As such, the first approach given above may sometimes be a better fit, since it rigidly enforces nesting semantics within any particular thread.

If you don't want to use the first approach, I would suggest that you should arrange your object so that Disposing an instance associated with a guarded resource will invalidate all instances produced after it. To minimize the "delayed astonishment" factor, you might want to have the resource assign a thread affinity when the first wrapper object is created, and forbid access by any other thread until all wrappers are Disposed (you might also allow access if the thread that created the object no longer exists, if it seems sufficiently unlikely that the thread would have done anything bad to the object before its disappearance).