Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#19456 closed Bug (fixed)

SimpleLazyObject raises RuntimeError if __class__ accessed while tracing execution

Reported by: rouyrre+django@… Owned by: nobody
Component: Utilities Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I found the issue while trying to profile a django application with pycallgraph. The issue is referenced there: https://github.com/gak/pycallgraph/issues/92

The issue is reproduced with the following code:

import sys
from django.utils.functional import SimpleLazyObject

def tracer(frame, event, arg):
    frame.f_locals['self'].__class__

sys.settrace(tracer)
SimpleLazyObject(lambda x: x)
sys.settrace(None)

I wrote a failing test in tests/regressiontests/utils/functional.py. The issue is solved by defining _wrapped=None in the LazyObject class in django/utils/functional.py and this does not seems to break anything, all tests still pass.

Do you prefer one commit for test + fix as it's a small issue or separate commits? Is the test written in the right place?

I can make a pull request once this ticket is created (as the commit guidelines specify I need a ticket number in the first place to commit & reference my code). Thanks for any comment about the issue or how to proceed.

Change History (7)

comment:1 Changed 3 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by russellm

  • Triage Stage changed from Unreviewed to Ready for checkin
  • Type changed from Uncategorized to Bug

One commit for test + fix is fine.

comment:3 Changed 2 years ago by anonymous

comment:4 Changed 2 years ago by aaugustin

  • Component changed from Uncategorized to Utilities

comment:5 Changed 2 years ago by aaugustin

The test should:

  • either restore the existing trace function if there is one rather than disable it,
  • or not run at all when a trace functions is active.

Otherwise this looks good.

comment:6 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 9dc5702932a0031bc4fb5473f2cdccffc61dbe30:

Fixed #19456 -- Avoid infinite recursion when tracing LazyObject.init.

Thanks blaze33 for the patch.

comment:7 Changed 2 years ago by anonymous

Good catch about restoring the trace function!
Thanks for the merge.

Note: See TracTickets for help on using tickets.
Back to Top