Exception state lives too long in 3.0

Python/ceval.c

 
 /* Execute compiled code */
 
 /* XXX TO DO:
    XXX speed up searching for keywords by using a dictionary
    XXX document it!
    */
 
 /* enable more aggressive intra-module optimizations, where available */
 #define PY_LOCAL_AGGRESSIVE
@@ skipping 1458 matching lines @@
                                 w = POP();
                                 u = POP();
                                 PyErr_Restore(v, w, u);
                                 why = WHY_RERAISE;
                                 break;
                         }
                         else if (v != Py_None) {
                                 PyErr_SetString(PyExc_SystemError,
                                         "'finally' pops bad exception");
                                 why = WHY_EXCEPTION;
+                        }
+                        /*
+                                Make sure the exception state is cleaned up before
+                                the end of an except block. This ensures objects
+                                referenced by the exception state are not kept
+                                alive too long.
+                                See #2507.

[jhylton, 2008/05/06 12:59:48]

We don't have a lot of comments that refer to specific bug reports.  I think
it's more common to refer to the bug in the checkin message, but omit it from
the code. 

[Antoine Pitrou, 2008/05/06 15:19:41]

On 2008/05/06 12:59:48, jhylton wrote:
> We don't have a lot of comments that refer to specific bug reports.  I think
> it's more common to refer to the bug in the checkin message, but omit it from
> the code. 

But is it a problem? Mentioning the bug number makes it much easier for a reader
to figure out why this code snippet is there. You can't rely on "svn blame" to
give you the right answer because the snippet might have been modified later for
whatever reason.

+                        */

[Antoine Pitrou, 2008/05/02 22:17:07]

Hmm, it looks like I have gone past the 79-column limit in this comment block.

+                        if (tstate->frame->f_exc_type != NULL)
+                                reset_exc_info(tstate);
+                        else {
+                                assert(tstate->frame->f_exc_value == NULL);
+                                assert(tstate->frame->f_exc_traceback == NULL);

[jhylton, 2008/05/06 12:59:48]

I'd move the asserts outside of the else.  They should always be true,
regardless of whether reset_exc_info() called.

[Antoine Pitrou, 2008/05/06 15:19:41]

On 2008/05/06 12:59:48, jhylton wrote:
> I'd move the asserts outside of the else.  They should always be true,
> regardless of whether reset_exc_info() called.

Well I've copied the code snippet from the end of the eval function, where
similar cleanup is done. I'm not sure why the original author felt the need to
add the asserts in the first place.

                         }
                         Py_DECREF(v);
                         break;
 
                 case LOAD_BUILD_CLASS:
                         x = PyDict_GetItemString(f->f_builtins,
                                                  "__build_class__");
                         if (x == NULL) {
                                 PyErr_SetString(PyExc_ImportError,
                                                 "__build_class__ not found");
@@ skipping 2542 matching lines @@
                         Py_DECREF(l);
                         return NULL;
                 }
                 PyList_SetItem(l, i, x);
         }
         return l;
 #endif
 }
 
 #endif