gh-130851: Only intern constants of types generated by the compiler #130901
Conversation
The free threading build interns and immortalizes most constants generated by the bytecode compiler. However, users can construct their own code objects with arbitrary constants. Don't intern or immortalize these objects if they are not a type that we know how to handle. This fixes a refleak failure in the recently added `test_code.test_unusual_constants` test. It also fixes a potential crash that could happen when we try to destory a immortalized object on interpreter shutdown.
colesbury
added
skip news
🔨 test-with-refleak-buildbots
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 5c3dd1c 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130901%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
colesbury
changed the title
|
@Yhg1s - The first attempt (#130853) had a few problems that this addresses:
Lines 2730 to 2733 in a025f27 This changes the behavior so that we now only intern and immortalize code constants that are instances of types that the bytecode compiler generates. |
|
The |
| @@ -241,8 +278,9 @@ intern_constants(PyObject *tuple, int *modified) | |||
|
|
|||
| // Intern non-string constants in the free-threaded build | |||
| _PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET(); | |||
| if (!_Py_IsImmortal(v) && !PyCode_Check(v) && | |||
| !PyUnicode_CheckExact(v) && !tstate->suppress_co_const_immortalization) | |||
| if (!_Py_IsImmortal(v) && !PyUnicode_CheckExact(v) && | |||
There was a problem hiding this comment.
Is the explicit check for unicode necessary, given that should_immortalize_constant also checks that? (For performance, perhaps?)
There was a problem hiding this comment.
The checks were in both places because I wrote should_immortalize_constant recursively for tuples, sets, and slices.
I've changed should_immortalize_constant so that it's no longer recursive and instead interns and immortalizes tuples, set, and slices if we've already immortalized their elements. That should still end up interning and immortalizing the same objects, but avoids the seemingly redundant checks.
|
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Sorry, @colesbury, I could not cleanly backport this to |
…he compiler (pythonGH-130901) The free-threading build interns and immortalizes most constants generated by the bytecode compiler. However, users can construct their own code objects with arbitrary constants. We should not intern or immortalize these objects if they are not of a type that we know how to handle. This change fixes a reference leak failure in the recently added `test_code.test_unusual_constants` test. It also addresses a potential crash that could occur when attempting to destroy an immortalized object during interpreter shutdown. (cherry picked from commit 12db452) Co-authored-by: Sam Gross <[email protected]>
|
GH-130953 is a backport of this pull request to the 3.13 branch. |
…piler (GH-130901) (#130953) The free-threading build interns and immortalizes most constants generated by the bytecode compiler. However, users can construct their own code objects with arbitrary constants. We should not intern or immortalize these objects if they are not of a type that we know how to handle. This change fixes a reference leak failure in the recently added `test_code.test_unusual_constants` test. It also addresses a potential crash that could occur when attempting to destroy an immortalized object during interpreter shutdown. (cherry picked from commit 12db452)
The free threading build interns and immortalizes most constants generated by the bytecode compiler. However, users can construct their own code objects with arbitrary constants. Don't intern or immortalize these objects if they are not a type that we know how to handle.
This fixes a refleak failure in the recently added
test_code.test_unusual_constantstest.It also fixes a potential crash that could happen when we try to destory a immortalized object on interpreter shutdown.