Is RAII safe to use in C#? And other garbage collecting languages?

Question

I was making an RAII class that takes in a System.Windows.Form control, and sets its cursor. And in the destructor it sets the cursor back to what it was.

But is this a bad idea? Can I safely rely that the destructor will be called when objects of this class go out of scope?

Answer 1

This is a very, very bad idea.

Finalizers are not called when variables go out of scope. They're called at some point before the object is garbage collected, which could be a long time later.

Instead, you want to implement IDisposable, and then callers can use:

using (YourClass yc = new YourClass())
{
    // Use yc in here
}

That will call Dispose automatically.

Finalizers are very rarely necessary in C# - they're only required when you directly own an unmanaged resource (e.g. a Windows handle). Otherwise, you typically have some managed wrapper class (e.g. FileStream) which will have a finalizer if it needs one.

Note that you only need any of this when you have resources to be cleaned up - most classes in .NET don't implement IDisposable.

EDIT: just to respond to the comment about nesting, I agree it can be a bit ugly, but you shouldn't need using statements very often in my experience. You can also nest usings vertically like this, in the case where you've got two directly adjacent ones:

using (TextReader reader = File.OpenText("file1.txt"))
using (TextWriter writer = File.CreateText("file2.txt"))
{
    ...
}

Answer 2

Use the IDisposable pattern if you want to do RAII-like things in C#

Answer 3

You know, a lot of smart people say "use IDisposable if you want to implement RAII in C#", and I just don't buy it. I know I'm in the minority here, but when I see "using(blah) { foo(blah); }" I automatically think "blah contains an unmanaged resource that needs to be cleaned up aggressively as soon as foo is done (or throws) so that someone else can use that resource". I do not think "blah contains nothing interesting but there's some semantically important mutation that needs to happen, and we're going to represent that semantically important operation by the character '}' " -- some mutation like some stack has to be popped or some flag has to be reset or whatever.

I say that if you have a semantically important operation that has to be done when something completes, we have a word for that and the word is "finally". If the operation is important, then it should be represented as a statement that you can see right there and put a breakpoint on, not an invisible side effect of a right-curly-brace.

So let's think about your particular operation. You want to represent:

var oldPosition = form.CursorPosition;
form.CursorPosition = newPosition;
blah;
blah;
blah;
form.CursorPosition = oldPosition;

That code is perfectly clear. All the side effects are right there, visible to the maintenance programmer who wants to understand what your code is doing.

Now you have a decision point. What if blah throws? If blah throws then something unexpected happened. You now have no idea whatsoever what state "form" is in; it might have been code in "form" that threw. It could have been halfway through some mutation and is now in some completely crazy state.

Given that situation, what do you want to do?

1) Punt the problem to someone else. Do nothing. Hope that someone else up the call stack knows how to deal with this situation. Reason that the form is already in a bad state; the fact that its cursor is not in the right place is the least of its worries. Don't poke at something that is already so fragile, it's reported an exception once.

2) Reset the cursor in a finally block and then report the problem to someone else. Hope -- with no evidence whatsoever that your hope will be realized -- that resetting the cursor on a form that you know is in a fragile state will not itself throw an exception. Because, what happens in that case? The original exception, which perhaps someone knew how to handle, is lost. You've destroyed information about the original cause of the problem, information which might have been useful. And you've replaced it with some other information about a cursor-moving failure which is of use to no one.

3) Write complex logic that handles the problems of (2) -- catch the exception, and then attempt to reset the cursor position in a separate try-catch-finally block which suppresses the new exception and re-throws the original exception. This can be tricky to get right, but you can do it.

Frankly, my belief is that the correct answer is almost always (1). If something horrid went wrong, then you cannot safely reason about what further mutations to the fragile state are legal. If you don't know how to handle the exception then GIVE UP.

(2) is the solution that RAII with a using block gives you. Again, the reason we have using blocks in the first place is to aggressively clean up vital resources when they can no longer be used. Whether an exception happens or not, those resources need to be cleaned up rapidly, which is why "using" blocks have "finally" semantics. But "finally" semantics are not necessarily appropriate for operations which are not resource-cleanup operations; as we've seen, program-semantics-laden finally blocks implicitly make the assumption that it is always safe to do the cleanup; but the fact that we're in an exception handling situation indicates that it might not be safe.

(3) sounds like a lot of work.

So in short, I say stop trying to be so smart. You want to mutate the cursor, do some work, and unmutate the cursor. So write three lines of code that do that, done. No fancy-pants RAII is necessary; it just adds unnecessary indirection, it makes it harder to read the program, and it makes it potentially more brittle in exceptional situations, not less brittle.

Answer 4

Edit: Clearly Eric and I are at a disagreement on this usage. :o

You can use something like this:

public sealed class CursorApplier : IDisposable
{
    private Control _control;
    private Cursor _previous;

    public CursorApplier(Control control, Cursor cursor)
    {
        _control = control;
        _previous = _control.Cursor;
        ApplyCursor(cursor);
    }

    public void Dispose()
    {
        ApplyCursor(_previous);
    }

    private void ApplyCursor(Cursor cursor)
    {
        if (_control.Disposing || _control.IsDisposed)
            return;

        if (_control.InvokeRequired)
            _control.Invoke(new MethodInvoker(() => _control.Cursor = cursor));
        else
            _control.Cursor = cursor;
    }
}

// then...

using (new CursorApplier(control, Cursors.WaitCursor))
{
    // some work here
}