[issue42482] TracebackException should not hold reference to the exception traceback

[Irit Katriel]

Change by Irit Katriel :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[miss-islington]

miss-islington  added the comment:


New changeset 40b92f1cc06f9aaba813ae38266f424e0969b089 by Miss Islington (bot) 
in branch '3.9':
[3.9] bpo-42482: remove reference to exc_traceback from TracebackException 
(GH-23531) (GH-23578)
https://github.com/python/cpython/commit/40b92f1cc06f9aaba813ae38266f424e0969b089


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[miss-islington]

miss-islington  added the comment:


New changeset 1cc5c943c007b92116f06b3ec8e71f2a510d1898 by Miss Islington (bot) 
in branch '3.8':
bpo-42482: remove reference to exc_traceback from TracebackException (GH-23531)
https://github.com/python/cpython/commit/1cc5c943c007b92116f06b3ec8e71f2a510d1898


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[miss-islington]

miss-islington  added the comment:


New changeset 427613f005f0f412d12f0d775d2b609bae0ae1ad by Irit Katriel in 
branch 'master':
bpo-42482: remove reference to exc_traceback from TracebackException (GH-23531)
https://github.com/python/cpython/commit/427613f005f0f412d12f0d775d2b609bae0ae1ad


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[miss-islington]

Change by miss-islington :


--
pull_requests: +22458
pull_request: https://github.com/python/cpython/pull/23579

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 5.0 -> 6.0
pull_requests: +22457
pull_request: https://github.com/python/cpython/pull/23578

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[Steven D'Aprano]

Steven D'Aprano  added the comment:

Re-adding older versions.

--
versions: +Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[Irit Katriel]

Irit Katriel  added the comment:

I've added the refcount check that Victor suggested.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[STINNER Victor]

STINNER Victor  added the comment:

I consider PR 23531 change as a bugfix and IMO it's fine to backport it.

TracebackException docstring is quite explicit:

The traceback module captures enough attributes from the original exception
to this intermediary form to ensure that no references are held, while
still being able to fully print or format it.

This issue shows that there was a bug in the implementation.

--

The unit test should check the ref count of the exception and the exception 
traceback, to check functionally that no strong reference is hold.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[Guido van Rossum]

Guido van Rossum  added the comment:

For background, see 
https://github.com/iritkatriel/cpython/pull/3#issuecomment-734640036 -- it 
seems the link to exc_traceback was added with little concern for the original 
design of TracebackExceptionGroup.

The question is, can we get rid of it, even though it's been undocumented.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[Eric V. Smith]

Change by Eric V. Smith :


--
nosy: +eric.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[Irit Katriel]

Irit Katriel  added the comment:

>From the TracebackException docstring:

"The traceback module captures enough attributes from the original exception to 
this intermediary form to ensure that no references are held, while still being 
able to fully print or format it."

> If the tracebacks are different, they're not exactly equivalent.

The formatted form is identical, so from TracebackException's POV they are 
equivalent. 

> If we did accept this patch, would that mean there would no longer be any 
> need to delete the exception variable at the end of an `except` block?

No, it's not related to that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[Steven D'Aprano]

Steven D'Aprano  added the comment:

This would be a change of behaviour, and 3.8 and 3.9 are in feature freeze, so 
we could only add it in 3.10.

You say:

"it's supposed to capture the output without holding references to real things"

Is this requirement documented somewhere, or is it just your preference?

"it makes comparison wrong for equivalent exceptions"

If the tracebacks are different, they're not exactly equivalent.

If we did accept this patch, would that mean there would no longer be any need 
to delete the exception variable at the end of an `except` block?

--
nosy: +steven.daprano
versions:  -Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[Irit Katriel]

Change by Irit Katriel :


--
keywords: +patch
pull_requests: +22413
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/23531

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[Irit Katriel]

Change by Irit Katriel :


--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42482] TracebackException should not hold reference to the exception traceback

[Irit Katriel]

New submission from Irit Katriel :

TracebackException holds a reference to the exc_traceback, which is wrong 
because (1) it's supposed to capture the output without holding references to 
real things. (2) it makes comparison wrong for equivalent exceptions, as in 
this example:

--
import sys
import traceback

excs = []
for _ in range(2):
try:
1/0
except:
excs.append(traceback.TracebackException(*sys.exc_info()))

print('formats equal: ', list(excs[0].format()) == list(excs[0].format()))
print('excs equal: ', excs[0] == excs[1])
excs[0].exc_traceback = excs[1].exc_traceback = None
print('excs equal w/o exc_traceback: ', excs[0] == excs[1])

--
Output:

formats equal:  True
excs equal:  False
excs equal w/o exc_traceback:  True

--


The good news is that it's only used to check for non-None (added here 
https://bugs.python.org/issue24695) so should be easy to remove.

--
components: Library (Lib)
messages: 381938
nosy: iritkatriel
priority: normal
severity: normal
status: open
title: TracebackException should not hold reference to the exception traceback
type: behavior
versions: Python 3.10, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com