classification
Title: TracebackException should not hold reference to the exception traceback
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eric.smith, gvanrossum, iritkatriel, miss-islington, steven.daprano, vstinner
Priority: normal Keywords: patch

Created on 2020-11-27 10:11 by iritkatriel, last changed 2020-12-04 22:19 by iritkatriel. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 23531 merged iritkatriel, 2020-11-27 10:49
PR 23578 merged miss-islington, 2020-12-01 01:35
PR 23579 merged miss-islington, 2020-12-01 01:35
Messages (10)
msg381938 - (view) Author: Irit Katriel (iritkatriel) * (Python triager) Date: 2020-11-27 10:11
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.
msg381945 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2020-11-27 11:54
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?
msg381946 - (view) Author: Irit Katriel (iritkatriel) * (Python triager) Date: 2020-11-27 12:05
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.
msg381956 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-11-27 16:52
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.
msg381959 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-27 19:30
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.
msg381962 - (view) Author: Irit Katriel (iritkatriel) * (Python triager) Date: 2020-11-27 20:14
I've added the refcount check that Victor suggested.
msg381969 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2020-11-27 22:05
Re-adding older versions.
msg382211 - (view) Author: miss-islington (miss-islington) Date: 2020-12-01 01:35
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
msg382212 - (view) Author: miss-islington (miss-islington) Date: 2020-12-01 01:53
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
msg382529 - (view) Author: miss-islington (miss-islington) Date: 2020-12-04 20:57
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
History
Date User Action Args
2020-12-04 22:19:54iritkatrielsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-12-04 20:57:46miss-islingtonsetmessages: + msg382529
2020-12-01 01:53:47miss-islingtonsetmessages: + msg382212
2020-12-01 01:35:56miss-islingtonsetmessages: + msg382211
2020-12-01 01:35:54miss-islingtonsetpull_requests: + pull_request22458
2020-12-01 01:35:45miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request22457
2020-11-27 22:05:41steven.dapranosetmessages: + msg381969
versions: + Python 3.8, Python 3.9
2020-11-27 20:14:30iritkatrielsetmessages: + msg381962
2020-11-27 19:31:00vstinnersetmessages: + msg381959
2020-11-27 16:52:44gvanrossumsetmessages: + msg381956
2020-11-27 14:25:26eric.smithsetnosy: + eric.smith
2020-11-27 12:05:40iritkatrielsetmessages: + msg381946
2020-11-27 11:54:49steven.dapranosetnosy: + steven.daprano

messages: + msg381945
versions: - Python 3.8, Python 3.9
2020-11-27 10:49:31iritkatrielsetkeywords: + patch
stage: patch review
pull_requests: + pull_request22413
2020-11-27 10:14:07vstinnersetnosy: + vstinner
2020-11-27 10:13:28iritkatrielsetnosy: + gvanrossum
2020-11-27 10:11:08iritkatrielcreate