Udostępnij za pośrednictwem


catch considered harmful

Spot the bug:

void CFoo::Bar() {

    m_array1[m_i++] = null;

    m_array2[m_j++] = null;

}

I’ll give you a hint – it relates to my last posting about “i = i +1;” being a bug.

One answer was to let the increment overflow.  Well in that case the bug seems pretty obvious.  Another answer is that you just have to be really careful.  Nobody wants to be careful any more when programming.  The remaining reasonable option was that integer overflows raise exceptions.

Ok, let’s explore that option.  Let’s say that the second of the two increments throws.  As long as the exception isn’t caught, everything’s OK – the failure was critical, the process terminated without any code attempting to rely on the (invalid) invariants in the CFoo object…

But why throw an exception if you don’t expect it to be caught?  Shouldn’t you call exit()/TerminateProcess()/whatever to just terminate the process?

So if you’re interested in throwing the exception you maybe should have written:

void CFoo::Bar() {

    int i = m_i++;

    try {

    Object o1 = m_array1[i];

        try {

            m_array1[i] = null;

            int j = m_j++;

            try {

                m_array2[i] = null;

            } catch {

                m_j = j;

                throw;

            }

        } catch {

            m_array1[i] = o1;

            throw;

        }

    } catch {

        m_i = i;

        throw;

    }

}

Well that’s still not quite right either; both GetValue and SetValue on System.Array may throw exceptions so even the recovery code may fail in which case the global invariants were not restored.  C++ has a hope of getting this right; let’s assume that the types are a value type with overloaded operators to throw on overflow…

void CFoo::Bar() {

    CResetter<CSizeT> r1(m_i);

    CResetter<CSizeT> r2(m_j);

    CObject *pArray1 = m_array1; // could throw

    CObject *pArray2 = m_array2; // could throw

    CResetter<CObject> r3(pArray1[m_i]);

    CResetter<CObject> r4(pArray2[m_j]);

    pArray1[m_i++] = null;

    pArray2[m_j++] = null;

    r1.Disarm(); r2.Disarm();

    r3.Disarm(); r4.Disarm();

}

Who writes code like that?  Who wants to write code like that?  There’s actually no way to do this “right” in the current CLR class library design since there’s no operator on System.Array that returns a reference which is the key to being able to do this in C++.

If someone catches the exception raised in the first case, whose bug is it?  The implementor of CFoo might say “well, I don’t expect anyone to catch exceptions my code throws” but the fact is that there are a lot of try / catch blocks sitting in interesting places like thread pools etc.

People mistakenly believe that there are kinds of exceptions which are uncatchable.  The truth is that there is a contract from the thrower through all the intervening frames to the catcher.  Anyone writing reusable code needs to understand this contract but do we really expect all the code in the world to be this rigorous and convoluted?

Note that the thread pool’s catch (Exception e) causes intervening unwinders to run, even if the exception is to be later propagated.  Running any code while invariants are false is dangerous at best.

Most of the exception literature came from people working in the areas of functional languages.  In a functional language this issue is moot since there weren't persistent side effects.  (I also worked on a programming environment once where each frame was a nested transaction and unwinding through a frame typically just rolled back the appropriate nested transaction.  But nobody wants to pay that kind of per-frame costs.)

I come back to postulate 1: programming is hard.  Programming shared components is harder.

Comments

  • Anonymous
    February 17, 2004
    The comment has been removed

  • Anonymous
    February 18, 2004
    The comment has been removed

  • Anonymous
    February 18, 2004
    The comment has been removed

  • Anonymous
    February 18, 2004
    The comment has been removed

  • Anonymous
    February 19, 2004
    Bunch of misconceptions here:

    1. The contract for (most) native APIs in Windows is that you may not catch any exceptions which propagate through the native API call frame. Yes, some people do it and sometimes it works but often it doesn't and when it doesn't, it won't get fixed. There are native APIs which have made code changes so that you can catch exceptions which pass through them but these are vastly in the minority and I won't name them on principle.

    2. If you can't deal with process termination, you have a bug in your code. There are many user mode conditions which will immediately and unilaterally terminate the process which you cannot easily guard against. (Easy case is touching your thread guard page without resetting it.)

    3. There is great internal debate about the utility of app domains. The debate goes something like this: "if all the code is managed and well mannered, app domains are sufficient" vs. "except for the cases in the real world where they aren't. Being in the same process is an optimization which needs to be dialable."

    This isn't meant to be a CLR issue. This is a generic programming one. The CLR has the notable problem that since there are no throw specifications, no reference returning members (I don't know if they even have reference types per se in the C++ sense), no conventions around the use of a non-throwing primitive like swap() etc. it's subject to some basic correctness limitations. But the same is true of most of these modern higher level languages which haven't yet been used to run critical software in stressful environments.

    The whole point actually of this thread of issues is that in the move towards higher level programming we seem to have forgotten or lost some of the basics.

  • Anonymous
    February 23, 2004
    Cool, CResetter, you don't need the Disarm calls though, what you do is in the destructor you check if an exception is going by, I forget the standard library function that tells you this, and based on it you do the rollback or not (rollback if there is an exception, do nothing if there isn't an exception).

  • Anonymous
    March 22, 2004
    The comment has been removed

  • Anonymous
    March 22, 2004
    Integer overflow is the next round of exploitable security bugs as evidenced by some of the recent security bullitins(sp?).

    So you're still stuck. I agree that CResetter had to make a copy of the value; maybe there's an even more clever solution which involves a single construction of a value in the CResetter and then a Swap (and a Swap in the destructor).

    I'm actually more optimistic for C++ here than the other exception-friendly languages because C++ has (a) deterministic finalization and (b) reference types. Reference types seem to be the real key here to being able to do anything.

    One might argue that exception specifications are the final key for utility but the problem is that while necessary, they are not sufficient to demonstrate that you have done the proper level of unwinding based on all your potential error-exit paths. they tend to lead to big try/catch blocks to make the compiler quiet rather than encouraging higher quality code.

  • Anonymous
    March 23, 2004
    The comment has been removed

  • Anonymous
    June 04, 2006
    PingBack from http://codeka.com/blogs/index.php/dean/2006/06/05/integer_overflows_the_next_big_thing

  • Anonymous
    September 07, 2006
    PingBack from http://www.ljseek.com/on-c-error-handling_13761091.html

  • Anonymous
    December 29, 2007
    PingBack from http://cars.oneadayvitamin.info/?p=650

  • Anonymous
    January 04, 2008
    PingBack from http://birthdays.247blogging.info/?p=515

  • Anonymous
    April 21, 2008
    PingBack from http://famouspeoplesbirthdayblog.info/mgriers-weblog-catch-considered-harmful/

  • Anonymous
    June 17, 2009
    PingBack from http://pooltoysite.info/story.php?id=5932