Setting field value in try{} and calling base class in finally{}

Question

I was browsing .NET Framework source code trying to understand another issue and I saw this code (in PeerNearMe.cs from System.Net.PeerToPeer.Collaboration):

private bool m_Disposed; 

protected override void Dispose(bool disposing)
{ 
    if (!m_Disposed){
        try{
            m_Disposed = true;
        } 
        finally{
            base.Dispose(disposing); 
        } 
    }
}

Is there any reason to put variable assignment in try block? May it fail in any way with an exception?! At first I thought it's because finally will be executed even if thread is aborted with Thread.Abort():

Unexecuted finally blocks are executed before the thread is aborted.

In this case I'd expect try block to include all method body:

try {
    if (!m_disposed)
        m_disposed = true;
}
finally {
    base.Dispose(disposing)
}

However they also says:

The thread that calls Abort might block if the thread that is being aborted is in a protected region of code, such as a catch block, finally block, or constrained execution region. If the thread that calls Abort holds a lock that the aborted thread requires, a deadlock can occur.

IMO calling a base class virtual method is little bit a jump in the dark in this case.

In short: what's this point of that code? What it really tries to implement? If it's because of Thread.Abort() then isn't it just wrong?

EDIT: if it's really just because of Thread.Abort() then it doesn't do its job if abort happens after if (!m_Disposed) { but before try {. Note that Dispose() must be resilient to multiple calls and simply do nothing (regardless when it's called).

Answer 1

The only thing that can happen is an asynchronous exception - Thread.Abort is one example, but there's also the likes of Thread.Interrupt or OutOfMemoryException.

Your suggestion actually breaks the code, because you call base.Dispose regardless of whether the instance was already disposed or not - that's not the intention.

Now, Thread.Abort should only be used when terminating an application domain - so you don't care whether the m_disposed = true succeeds or not, the domain is going to be torn down soon anyway. However, you still care about releasing any native resources, because those are usually tied to the process, not the application domain (and sometimes, they even transcend processes or whole machines).

Code in finally gets a chance to run even in the middle of a Thread.Abort - there's no other way to ensure code runs during an asynchronous exception. By calling base.Dispose in a finally clause, you ensure that it at least gets a chance to execute, and that it will not be terminated in the middle of operation (though note that there's a fixed time limit for all the finally clauses that execute - you don't want to do anything complex in a finally).

Now, in this particular case, there isn't a real reason to do this - the base class doesn't do anything either. So it's probably just a common Dispose pattern the team uses :) Since Dispose is designed for deterministic release of native resources, it's perfectly safe to call it in a finally clause - it's not supposed to do any work, just release native resources. Of course, Dispose is frequently abused - but then you just reap what you've sown.

Finally, don't forget that this is also exactly what a using clause does, so if you're using using, you're already running the Dispose method in a finally clause!

using (var bmp = new Bitmap())
{
  ...
}

Is translated to the equivalent of

Bitmap bmp = null;
try
{
  bmp = new Bitmap();

  ...
}
finally
{
  if (bmp != null) bmp.Dispose();
}

All in all, there's nothing suspicious about that implementation, really :)

Answer 2

I'm not sure about why its in a try as it's only assigning a variable, maybe someone else can tell us this.

I would say that your expectation would act differently though. Putting the whole try finally inside the if(!m_disposed) means that the object won't call Dispose if that m_disposed is true whereas your expectation would call dispose regardless of m_disposed value.