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.
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
gh-113781: Silence AttributeError in warning module during Python finalization #113813
gh-113781: Silence AttributeError in warning module during Python finalization #113813
Conversation
…on finalization The tracemalloc module can already be cleared.
I was not able to write a simple test for this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! A thought.
Lib/warnings.py
Outdated
tb = tracemalloc.get_object_traceback(msg.source) | ||
except Exception: | ||
# When a warning is logged during Python shutdown, tracemalloc | ||
# and the import machinery don't work anymore | ||
tracing = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be True
? Like above at line 61. And note below at line 88/89, the "enable tracemalloc" message is appended if tracing is false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that except Exception
was added because the tracemalloc
module was optional. In that case it is reasonable that we should not suggest to use the tracemalloc
module.
But in fact, it most likely fails because the import system is already broken at Python shutdown, so no new modules can be imported. And if we are in the else
block, then either the import system works, or tracemalloc
was imported earlier. Can we suggest to enable it? Who knows. I initially thought that we can, running Python with -X tracemalloc
seems worked in these examples. But I failed to write tests that do not involve running tests via unittest
(or even libregrtest
), so I am now not sure that it always works. So perhaps it is safer to set tracing = True
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tracing
variable is a bit of a misnomer -- it's just used to suppress the "Enable tracemalloc" message. Maybe reverse the sense and rename it to "suggest_tracemalloc"? Then we'd get
# If we're already tracing, don't suggest to enable tracemalloc
suggest_tracemalloc = not tracemalloc.is_tracing()
The idea being that if we're already tracing the suggestion is redundant. (See the original PR gh-10486 where it was introduced.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go for it!
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
…on finalization (pythonGH-113813) The tracemalloc module can already be cleared. (cherry picked from commit 0297418) Co-authored-by: Serhiy Storchaka <[email protected]>
GH-113873 is a backport of this pull request to the 3.12 branch. |
…on finalization (pythonGH-113813) The tracemalloc module can already be cleared. (cherry picked from commit 0297418) Co-authored-by: Serhiy Storchaka <[email protected]>
GH-113874 is a backport of this pull request to the 3.11 branch. |
…hon finalization (GH-113813) (GH-113873) The tracemalloc module can already be cleared. (cherry picked from commit 0297418) Co-authored-by: Serhiy Storchaka <[email protected]>
…hon finalization (GH-113813) (GH-113874) The tracemalloc module can already be cleared. (cherry picked from commit 0297418) Co-authored-by: Serhiy Storchaka <[email protected]>
The tracemalloc module can already be cleared.
tracemalloc.is_tracing
#113781