Skip to content

bpo-31321: Fix traceback.clear_frames() #3262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vstinner wants to merge 1 commit into from
Closed

bpo-31321: Fix traceback.clear_frames() #3262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 61 additions & 14 deletions Lib/test/test_traceback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,24 @@
test_tb = namedtuple('tb', ['tb_frame', 'tb_lineno', 'tb_next'])


# Variable and functions used to test clear_frames()
clear_frames_err = None


def clear_frames_func1():
x = 1
clear_frames_func2()


def clear_frames_func2():
global clear_frames_err
z = 3
try:
raise ValueError()
except Exception as exc:
clear_frames_err = exc


class TracebackCases(unittest.TestCase):
# For now, a very minimal set of tests. I want to be sure that
# formatting of SyntaxErrors works based on changes for 2.1.
Expand Down Expand Up @@ -751,29 +769,58 @@ class MiscTracebackCases(unittest.TestCase):
# Check non-printing functions in traceback module
#

def test_clear(self):
def test_clear_frames(self):
err = None

def outer():
x = 1
middle()
def middle():
y = 2
inner()
def inner():
i = 1
1/0

try:
outer()
except:
type_, value, tb = sys.exc_info()

# Initial assertion: there's one local in the inner frame.
inner_frame = tb.tb_next.tb_next.tb_next.tb_frame
self.assertEqual(len(inner_frame.f_locals), 1)
nonlocal err
z = 3
try:
1/0
except Exception as exc:
err = exc

def get_locals(tb):
flocals = {}
frame = tb.tb_frame
while frame is not None:
name = frame.f_code.co_name
if name in ('outer', 'middle', 'inner'):
flocals[name] = dict(frame.f_locals)
frame = frame.f_back
return flocals

# Indirectly call inner() to set err
outer()
tb = err.__traceback__

# Initial assertion: all frames contains local variables
tb_locals = {
'outer': {'middle': middle, 'x': 1},
'middle': {'inner': inner, 'y': 2},
'inner': {'err': err, 'z': 3},
}
self.assertEqual(get_locals(tb), tb_locals)

# Clear traceback frames
traceback.clear_frames(tb)

# Local variable dict should now be empty.
self.assertEqual(len(inner_frame.f_locals), 0)
# Local variable dictionaries should now be empty.
tb_locals = {
'outer': {},
'middle': {},
'inner': {},
}
self.assertEqual(get_locals(tb), tb_locals)

# Explicitly break a reference cycle
err = None

def test_extract_stack(self):
def extract():
Expand Down
21 changes: 16 additions & 5 deletions Lib/traceback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,12 +211,23 @@ def extract_stack(f=None, limit=None):

def clear_frames(tb):
"Clear all references to local variables in the frames of a traceback."
seen = set()
while tb is not None:
try:
tb.tb_frame.clear()
except RuntimeError:
# Ignore the exception raised if the frame is still executing.
pass
frame = tb.tb_frame
while True:
Copy link
Member

@pitrou pitrou Sep 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is making clear_frames() a potentially O(n**2) operation. In practice, you don't need this, as tb.tb_next.tb_frame.f_back will be the same as tb.tb_frame (and so on). So the loops needn't be nested.

Copy link
Member

@pitrou pitrou Sep 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you can keep the nesting but mark frames as seen as you iterate on them, which will have the same effect while being more general (in case someone changes the traceback chain).

Copy link
Member Author

@vstinner vstinner Sep 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is making clear_frames() a potentially O(n**2) operation.

Sorry, I don't know well how traceback objects are created. I didn't even know that you can have two tracebacks objects linked together.

I added a "seen = set()" to prevent iterating twice on the same frame chain. Does it solve your O(n**2) issue?

I also used my example attached to the bpo to enhance the existing unit test.

Copy link
Member

@pitrou pitrou Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use while frame is not None here instead of having a separate test at the end of the loop.

key = id(frame)
if key in seen:
break
seen.add(key)

try:
frame.clear()
except RuntimeError:
# Ignore the exception raised if the frame is still executing.
pass
frame = frame.f_back
if frame is None:
break
tb = tb.tb_next


Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2017-09-01-16-51-04.bpo-31321.AXtvkT.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
traceback.clear_frames() now iterates on frames to clear all frames of each
traceback object, not only the first frame.