[Patches] Patch: AttributeError and NameError: second attempt.

[M.-A. Lemburg]

Nick Mathewson wrote:
> 
> "M.-A. Lemburg" <mal@lemburg.com> wrote:
>  [...]
> >The problem with storing objects in error objects is that they
> >can easily create (well) hidden circular references and cause
> >unwanted side-effects:
> >
> >1. A NameError that is being catched will
> >have a reference to the frame which currently executes (and this
> >references the variable holding the error object) -- the frame
> >will stay alive forever if the error object is not properly
> >cleaned up.
> 
> Do you mean 'caught', or 'cached'? 

I meant 'caught'.

> Catching is no problem, I think;
> I've experimented, and not been able to generate reference leaks with
> NameErrors.  Caching is a problem, however.  Perhaps rather than
> incorporating the frame as part of the error object, we should just
> pull it from the traceback when we try to print the error.

Right... that's the way to go. Besides frame objects are not
always available, e.g. C code raising an exception will
not have any frame object available.
 
> The only way that the error object won't get 'cleaned up', if I
> understand it, is if somebody stores a copy of the exception object
> for future reference.  I haven't seen a lot of code that does this
> with NameError, but I agree it would be a problem if anybody does
> this now.

Here's a common example:

try:
   blabla
except NameError, why:
   print why
# now why stays around and the frame object with it...

Or:

try:
   blabla
except:
   ec, eo = sys.exc_info[:2]
# the traceback is not put into a local on purpose, yet
# the frame is also stored in eo...
 
> >2. An AttributeError carrying around the object that caused
> >the error could cause finally-clauses to fail due to some
> >resource being kept alive or open.
> 
> Hm.  This is a valid concern.  I'm not sure what to do about it.
> Does anybody have any good suggestions?

Yes: don't include the object in the error object. Instead,
get all the information from the traceback object during
printing of the exception.
 
> >I'd suggest not adding too much logic to the error objects
> >themselves, but rather to the code writing the tracebacks.
> 
> I tried this approach (not putting logic in the error objects) with
> the first version of my patch, but it seems that my options are really
> limited: if I do the work when the exception is raised, then I'll
> incur a big performance hit for every AttributeError.  If I do the
> work when the exception is displayed (as I do now), then I think I
> really must keep a reference to the object around until it's
> displayed.
> 
> There _may_ exist a some-now-some-later approach, but I'm not sure
> how to divide the work.
> 
> >Since the above tricks are mainly intended to provide better
> >user feedback (which is implemented by writing a traceback),
> >this solves the problem without causing additional side-effects
> >or severe slow-downs. There are *very* many instances where
> >e.g. AttributeErrors are raised only to be catched and then
> >causing a different processing branch to be taken -- I would
> >strongly object if this action would slow down significantly
> >because I use this a lot !
> 
> This is related Guido's objection to the first version of my patch;
> the second version addresses this concern, but at the expense of
> putting adding a reference to the object to the AttributeError object.
> 
> BTW, to address your speed concerns (and my own curiousity) I tried a
> completely artificial benchmarkmark test, before and after my changes.
> ========================================
> benchmark 1: Does nothing but raise AttributeErrors.  Never prints them.
>    class E: pass
>    e = E()
>    for i in xrange(500000):
>       try:
>          e.notThere
>       except:
>          pass
> 
> modified python:          22.790u 0.030s

You mean CVS Python + your patch ? If yes, how can there be
a speedup of this amount ?

> cvs python, unmodified:   32.920u 0.000s
> python 1.6a2:             39.850u 0.060s
> python 1.5.2:             33.380u 0.020s
>
> Benchmark 2: Raises a bunch of exceptions, and _does_ simulate printing them.
>    class E: pass
>    e = E()
>    for i in xrange(500000):
>       try:
>          e.notThere
>       except AttributeError, z:
>          str(z)
> 
> modified python:         119.800u 0.080s
> cvs python, unmodified:   46.530u 0.550s
> python 1.6a2:             62.900u 0.050s
> python 1.5.2:             51.950u 0.020s
> ========================================
> Performance conclusions:
> 
> 1) Doing all the work at print time makes it much faster to raise exceptions.
> 2) Doing lots of work at print time makes it much slower to print exceptions.
> 3) For the load you describe, the way I've written it is better, assuming
>    that the reference issues can be resolved.

Note that you'll only print the exception in interactive sessions
or when writing it to log files. In both cases, performance is
not relevant, but information quality is. Only having the object
around doesn't always help, because there may be more context
needed in order to find the bug. The locals printout I'm using
has helped me quite a few times and is a great development
tool.

-- 
Marc-Andre Lemburg
______________________________________________________________________
Business:                                      http://www.lemburg.com/
Python Pages:                           http://www.lemburg.com/python/