gh-124785: re-work fix so tracerefs test passes - #124808
Conversation
Sorry, something went wrong.
| return (interp != main_interp && | ||
| !(interp->feature_flags & Py_RTFLAGS_MULTI_INTERP_EXTENSIONS)); |
There was a problem hiding this comment.
FYI, (not Py_RTFLAGS_MULTI_INTERP_EXTENSIONS) does not imply Py_RTFLAGS_USE_MAIN_OBMALLOC. Why not stick with Py_RTFLAGS_USE_MAIN_OBMALLOC?
Sorry, something went wrong.
There was a problem hiding this comment.
I'm looking at reload_singlephase_extension and when we can end up in the PyDict_Update case. I think it can only happen if Py_RTFLAGS_MULTI_INTERP_EXTENSIONS is set. If we do that dict update then we are sharing objects between interpreters.
Sorry, something went wrong.
Spelling fix. Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
|
For the backport to 3.12 and 3.13 we could make interned_strings_legacy a global var rather than part of the interpreter state structure (if that's a problem for maintaining the ABI). I think that's safe because only the main interpreter would use that global. That's assuming we think this change is appropriate as a backported fix. Another option would be GH-124796, simpler code but it leaks the immortal strings whereas this PR frees them (eventually). |
Sorry, something went wrong.
|
Closing this. After discussion with Eric Snow, I think his GH-124865 is the preferred fix. It has the advantage that it's just a relatively small change on top of GH-124646 (to fix the tracerefs issue) and it doesn't touch the ABI like this one does. So, it should be easier to backport. |
Sorry, something went wrong.
The previous fix to this bug caused some trace-refs tests to fail. Object references from the sub-interpreters were not being correctly accounted in the main interpreter. Re-work the fix so that the interned strings for sub-interpreters go into their own dict, interned_strings_legacy. That allows the main interpreter to clean them knowing that those specific strings have been allocated in sub-interpreters.