Is Microsoft IDisposable pattern actually correct?

Question

I've stumbled across Microsoft's recommended way to implement the IDisposable pattern many times, it's even present in Visual Studio as an "Implement Interface" option in the lamp icon menu. It looks like this:

// Override only if 'Dispose(bool disposing)' has code to free unmanaged resources
~Foo() {
    // Do not change this code.
    Dispose(calledByFinalizer: true);
}

public void Dispose() {
    // Do not change this code. 
    Dispose(calledByFinalizer: false);
    GC.SuppressFinalize(this);
}

// Put cleanup code here
protected virtual void Dispose(bool calledByFinalizer) {
    if (_disposed) return;

    if (!calledByFinalizer) { /* dispose managed objects */ }

    /* free unmanaged resources and set large fields to null */

    _disposed = true;
}

I refactored the suggested code a bit (because Dispose(bool disposing) can break someone's brain, and nested if's can break someone's eyes).

But I still have some questions on my mind:

1. It is assumed that the method will be called once. Then why is _disposed = true placed at the end of the method and not at the beginning? If IDisposable.Dispose() is called from different threads, then they can all bypass the if (_disposed) return; check and actually execute the method body twice. Why not do it like this:

    if (_disposed) return;
    else _disposed = true;

2. Why is protected virtual void Dispose(bool disposing) flagged as virtual? Any derived class does not have access to the _disposed field and can easily break its behavior. We can only mark as virtual the optional part where the derived class can do anything without calling base.Dispose():

~Foo() => FreeUnmanagedResources();

public void Dispose() {
    if (_disposed) return;
    else _disposed = true;

    DisposeManagedObjects();
    FreeUnmanagedResources();

    GC.SuppressFinalize(this);
}

protected virtual void DisposeManagedObjects() { }
protected virtual void FreeUnmanagedResources() { }

Answer 1

The pattern is correct, but assumes the worst case scenario, where you must also implement a finalizer. That is, if you need a finalizer you must also follow the entire pattern. However...

It's RARE to need a finalizer at all.

You only need a finalizer if you are creating the original managed wrapper for a brand new kind of unmanaged resource.

For example, let's say you create a brand new, never before seen database system. You want to provide a .Net ADO provider for this new kind of database, including the connections (it will inherit from DbConnection). The underlying network operations here will be an unmanaged resource, and there isn't already a finalizer to release them anywhere in your inhertitance tree. Therefore, you must implement your own finalizer.

On the other hand, if you are creating a wrapper object for your application to manage connections to an existing database type — just repacking (wrapping or inheriting) an existing SqlConnection, OleDbConnection, MySqlConnection, etc — then you should still implement IDisposable, but there is already a finalizer provided for the unmanaged resource and you don't need to write another.

It turns out, when you don't have a finalizer you can safely remove a lot of the code from the documented IDisposable pattern.

From Microsoft's perspective, it's better to encourage people to add a few extra harmless lines of code rather then let them miss something potentially important.