You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Interned strings are immortal, despite what the documentation says #113993
Interned strings are not immortal; you must keep a reference to the return value of intern() around to benefit from it.
However, they were made immortal in #19474 (Implement Immortal Objects -- implementation of PEP 683), without a documentation update. The 3.12 What's New also doesn't mention the change. [edit: it's now clear that the PR author (@eduardo-elizondo) intended it but the reviewer (@markshannon) did not] PEP-683 itself only mentions the change as a possible optimization.
Meanwhile, pathlibinterns every path segment it processes, presumably depending on the documented behaviour. In CPython's test suite, that causes names of temporary directories to leak (stealthily, since interned strings are excluded from the total refcount).
[edit: Worse, type_setattro, PyObject_SetAttr or PyDict_SetItemString immortalize keys, so strings used for key/attribute names this way are not reclaimed until interpreter shutdown.]
On the other hand, free-threading (PEP-703) seems to rely [edit: relies] on these being immortal.
What's the correct behaviour? [edit: specifically, in 3.12 and non-free-threading 3.13, should this change be reverted or documented?]
I would suggest that using "immortal" here could be confusing. The flagged checked by _Py_IsImmortal() is low-level implementation detail and should not be something that the library documentation discusses. Strings that you call intern() on may or may not have this flag set. Different Python runtimes might implement intern() some other way. The behavior that matters is that the string gets stored in an interned strings table and that future calls to intern() with the same string content will return an object with the same id(), even if the user program doesn't keep a reference to the string.
The behavior that matters is that the string gets stored in an interned strings table and that future calls to intern() with the same string content will return an object with the same id(), even if the user program doesn't keep a reference to the string.
No. That is not the behaviour on Python 3.11.
I get:
$ python3.11
Python 3.11.7 (main, Dec 18 2023, 00:00:00) [GCC 13.2.1 20231205 (Red Hat 13.2.1-6)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> id1 = id(sys.intern('some string'))
>>> other = sys.intern('other string')
>>> id2 = id(sys.intern('some string'))
>>> id1 == id2
False
IMO, the guarantee is that if two strings are interned, they can be compared by identity.
CPython uses this for a fast path in string comparison; so some users call intern on strings that are likely to be compared to one another -- even ones that srent expected to live until interpreter shutdown. We can say that's optmizing for a specific implementation, but it's common -- even pathlib and the compiler (e.g. eval) do it.
The new behaviour can leak strings that are not likely to be used again.
The the new string is allowed to reuse the old one's memory; I guess this is build-specific. What platform are you on?
Instead of other = sys.intern('other string'), try putting in gc.collect and something more intensive, like import asyncio?
@encukou just getting to this! You're right, as of now, all interned strings should be immortal, so the sys.intern docs should probably be updated to update that they are indeed immortal and the fact that we don't need to keep a reference to benefit from it anymore.
As for the leaks, as @erlend-aasland already called out, we have a PR to correctly clean them up in #113601!
The behavior that matters is that the string gets stored in an interned strings table and that future calls to intern() with the same string content will return an object with the same id(), even if the user program doesn't keep a reference to the string.
No. That is not the behaviour on Python 3.11.
Sorry, I was not very clear. I was describing the behaviour we have with Python 3.12 and we had with older versions of Python (version 2.2 and before, it seems). It looks like this commit changed it: 45ec02a.
Since 3.12 changed this, definitely the documentation should be updated. Further, I think we should have had a discussion and explicitly decide if this is desirable behaviour, since it reverts the effect of 45ec02a. Maybe there was one and I missed it? Perhaps free-threading doesn't require that every interned string become immortal but only some of them (e.g. symbols used by code objects etc). That might preserve free-threaded performance (e.g. interned strings shared between threads) while avoid the "leak" like encountered by pathlib (e.g. interned strings only used for a short time and then discarded).
as of now, all interned strings should be immortal
To be clear, I think this is bad. Some interned strings are temporary, and they now leak. They should be cleaned up sooner that at interpreter shutdown.
Reacted by Mark Shannon, Inada Naoki, Jeffrey Bosboom and Patrik Veselý
Yes, I agree this should be reverted in 3.12, and conditional on free threading in 3.13. That's my opinion as RM, but considering the discussions the DC had about the immortal objects PEP, I'm sure they all agree.
Unfortunately it's far from a straightforward revert.
Do you still want this in 3.12? Should I ask the SC?
Activity
nascheme commentedon Jan 12, 2024
I would suggest that using "immortal" here could be confusing. The flagged checked by
_Py_IsImmortal()is low-level implementation detail and should not be something that the library documentation discusses. Strings that you callintern()on may or may not have this flag set. Different Python runtimes might implementintern()some other way. The behavior that matters is that the string gets stored in an interned strings table and that future calls tointern()with the same string content will return an object with the sameid(), even if the user program doesn't keep a reference to the string.encukou commentedon Jan 15, 2024
No. That is not the behaviour on Python 3.11.
I get:
IMO, the guarantee is that if two strings are interned, they can be compared by identity.
CPython uses this for a fast path in string comparison; so some users call
internon strings that are likely to be compared to one another -- even ones that srent expected to live until interpreter shutdown. We can say that's optmizing for a specific implementation, but it's common -- evenpathliband the compiler (e.g.eval) do it.The new behaviour can leak strings that are not likely to be used again.
erlend-aasland commentedon Jan 15, 2024
No assertions triggered.
encukou commentedon Jan 15, 2024
The the new string is allowed to reuse the old one's memory; I guess this is build-specific. What platform are you on?
Instead of
other = sys.intern('other string'), try putting ingc.collectand something more intensive, likeimport asyncio?erlend-aasland commentedon Jan 15, 2024
I'm on macOS using the official build. Strange thing: I can reproduce the behaviour you're seeing in the REPL.
erlend-aasland commentedon Jan 15, 2024
Nope, cannot reproduce using
$ python3.11 ./test.pyas above.encukou commentedon Jan 15, 2024
Ah! Right, literal constants are part the code object, which does normally stay around. You'll need something that's not folded to a constant, like:
erlend-aasland commentedon Jan 15, 2024
Yes; thanks!
erlend-aasland commentedon Jan 15, 2024
See also #113601
eduardo-elizondo commentedon Jan 15, 2024
@encukou just getting to this! You're right, as of now, all interned strings should be immortal, so the
sys.interndocs should probably be updated to update that they are indeed immortal and the fact that we don't need to keep a reference to benefit from it anymore.As for the leaks, as @erlend-aasland already called out, we have a PR to correctly clean them up in #113601!
nascheme commentedon Jan 15, 2024
Sorry, I was not very clear. I was describing the behaviour we have with Python 3.12 and we had with older versions of Python (version 2.2 and before, it seems). It looks like this commit changed it: 45ec02a.
Since 3.12 changed this, definitely the documentation should be updated. Further, I think we should have had a discussion and explicitly decide if this is desirable behaviour, since it reverts the effect of 45ec02a. Maybe there was one and I missed it? Perhaps free-threading doesn't require that every interned string become immortal but only some of them (e.g. symbols used by code objects etc). That might preserve free-threaded performance (e.g. interned strings shared between threads) while avoid the "leak" like encountered by pathlib (e.g. interned strings only used for a short time and then discarded).
encukou commentedon Jan 16, 2024
@eduardo-elizondo
To be clear, I think this is bad. Some interned strings are temporary, and they now leak. They should be cleaned up sooner that at interpreter shutdown.
35 remaining items
[3.13] pythongh-113993: Don't immortalize in PyUnicode_InternInPlace;…
pythongh-113993: Allow interned strings to be mortal, and fix related…
pythongh-113993: For string interning, do not rely on (or assert) _Py…
pythongh-113993: Don't immortalize in PyUnicode_InternInPlace; keep i…
[3.13] gh-113993: Don't immortalize in PyUnicode_InternInPlace; keep …
[3.12] pythongh-113993: Allow interned strings to be mortal, and fix …
[3.12] pythongh-113993: Don't immortalize in PyUnicode_InternInPlace;…
encukou commentedon Aug 28, 2024
The 3.12 backport is at #123065.
@Yhg1s, re:
Unfortunately it's far from a straightforward revert.
Do you still want this in 3.12? Should I ask the SC?
[3.12] gh-113993: Make interned strings mortal (GH-120520, GH-121364, G…
[3.12] pythongh-113993: For string interning, do not rely on (or asse…
[3.12] gh-113993: For string interning, do not rely on (or assert) _P…
gh-113993: InternalDocs: Add String Interning to README (#127250)
pythongh-113993: InternalDocs: Add String Interning to README (python…