Description

colesbury

opened on Jul 25, 2024

Issue body actions

Bug report

Most static strings are interned during Python initialization in _PyUnicode_InitStaticStrings. However, the _Py_LATIN1_CHR characters (code points 0-255) are static, but not interned. They may be interned later while the Python is running. This can happen for various reasons, including calls to sys.intern.

This isn't thread-safe: it modifies the hashtable _PyRuntime.cached_objects.interned_strings, which is shared across threads and interpreters, without any synchronization.

It also can break the interning identity invariant. You can have a non-static, interned 1-characters string later shadowed by the global interning of the static 1-character string.

Suggestions

• The _PyRuntime.cached_objects.interned_strings should be immutable. We should not modify it after Py_Initialize() until shutdown (i.e., _PyUnicode_ClearInterned called from finalize_interp_types())
• The 1-character latin1 strings should be interned. This can either be by explicitly interning them during startup, or by handling 1-character strings specially in intern_common.

cc @encukou @ericsnowcurrently

Linked PRs

• gh-122291: Intern latin-1 one-byte strings at startup #122303
• [3.13] gh-122291: Intern latin-1 one-byte strings at startup (GH-122303) #122347

Reacted by Petr Viktorin

Activity

1. colesbury

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Jul 25, 2024
2. encukou commented on Jul 26, 2024

   

   encukou

   on Jul 26, 2024

   Member

   More actions

   Interning them makes sense. I'll send a PR.
3. encukou

   self-assigned this

   on Jul 26, 2024
4. encukou

   added a commit that references this issue on Jul 27, 2024

   gh-122291: Intern latin-1 one-byte strings at startup (GH-122303)

   bb09ba6
5. miss-islington

   added a commit that references this issue on Jul 27, 2024

   pythongh-122291: Intern latin-1 one-byte strings at startup (pythonGH…

   8b26a3b
6. encukou commented on Jul 27, 2024

   

   encukou

   on Jul 27, 2024

   Member

   More actions

   FWIW:

   This isn't thread-safe: it modifies the hashtable _PyRuntime.cached_objects.interned_strings, which is shared across threads and interpreters, without any synchronization.

   Yup, that's a bug, so the fix should be backported.

   It also can break the interning identity invariant. You can have a non-static, interned 1-characters string later shadowed by the global interning of the static 1-character string.

   Hmm, interning a short string should give you the static singleton.
   But the fix will definitely make this easier to follow, and harder to hide any bugs.
7. encukou commented on Jul 27, 2024

   

   encukou

   on Jul 27, 2024

   Member

   More actions

   interning a short string should give you the static singleton.

   Before the PR for this issue we had:
   

   cpython/Objects/unicodeobject.c

   Lines 15480 to 15493 in c086962

   	/* if it's a short string, get the singleton -- and intern it */
   	if (PyUnicode_GET_LENGTH(s) == 1 &&
   	PyUnicode_KIND(s) == PyUnicode_1BYTE_KIND) {
   	PyObject *r = LATIN1(*(unsigned char*)PyUnicode_DATA(s));
   	if (!PyUnicode_CHECK_INTERNED(r)) {
   	r = intern_static(interp, r);
   	}
   	Py_DECREF(s);
   	return r;
   	}
   	#ifdef Py_DEBUG
   	assert(!unicode_is_singleton(s));
   	#endif
8. encukou

   added a commit that references this issue on Jul 27, 2024

   [3.13] gh-122291: Intern latin-1 one-byte strings at startup (GH-122303…

   4092510
9. encukou

   added a commit that references this issue on Aug 16, 2024

   [3.12] pythongh-122291: Intern latin-1 one-byte strings at startup (p…

   1d4a531
10. colesbury commented on Oct 23, 2024

    

    colesbury

    on Oct 23, 2024

    ContributorAuthor

    More actions

    Looks like this has been fixed for a while.
11. colesbury

    closed this as completedon Oct 23, 2024

Issue actions