Fix dataclasses in subinterpreters

Fixes https://bugzilla.redhat.com/2252567

The added commit was merged to the 3.12 branch upstream.
This commit is contained in:
Miro Hrončok 2023-12-05 11:55:04 +01:00
commit c342899850
2 changed files with 387 additions and 1 deletions

View file

@ -0,0 +1,361 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 27 Nov 2023 16:51:12 -0700
Subject: [PATCH] 00411: gh-106931: Intern Statically Allocated Strings
Globally
We tried this before with a dict and for all interned strings.
That ran into problems due to interpreter isolation.
However, exclusively using a per-interpreter cache caused some inconsistency
that can eliminate the benefit of interning.
Here we circle back to using a global cache,
but only for statically allocated strings.
We also use a more-basic _Py_hashtable_t for that global cache instead of a dict.
Ideally we would only have the global cache, but the optional isolation
of each interpreter's allocator means that a non-static string object
must not outlive its interpreter.
Thus we would have to store a copy of each such interned string
in the global cache, tied to the main interpreter.
(cherry-picked from commit b72947a8d26915156323ccfd04d273199ecb870c)
(cherry-picked from commit 3e6b32875207c39325ed8eded5819cfa5dcebc82)
---
Include/cpython/unicodeobject.h | 4 +-
Include/internal/pycore_global_objects.h | 6 ++
Include/internal/pycore_hashtable.h | 1 +
Include/internal/pycore_runtime.h | 1 +
Include/internal/pycore_runtime_init.h | 1 +
Lib/test/test_sys.py | 41 +++++++++++
...-07-25-15-29-26.gh-issue-106931.kKU1le.rst | 3 +
Objects/unicodeobject.c | 72 ++++++++++++++++++-
Python/hashtable.c | 7 ++
Tools/build/deepfreeze.py | 2 +
10 files changed, 134 insertions(+), 4 deletions(-)
create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-07-25-15-29-26.gh-issue-106931.kKU1le.rst
diff --git a/Include/cpython/unicodeobject.h b/Include/cpython/unicodeobject.h
index 3394726dff..f177cd9e2a 100644
--- a/Include/cpython/unicodeobject.h
+++ b/Include/cpython/unicodeobject.h
@@ -140,9 +140,11 @@ typedef struct {
and the kind is PyUnicode_1BYTE_KIND. If ascii is set and compact is
set, use the PyASCIIObject structure. */
unsigned int ascii:1;
+ /* The object is statically allocated. */
+ unsigned int statically_allocated:1;
/* Padding to ensure that PyUnicode_DATA() is always aligned to
4 bytes (see issue #19537 on m68k). */
- unsigned int :25;
+ unsigned int :24;
} state;
} PyASCIIObject;
diff --git a/Include/internal/pycore_global_objects.h b/Include/internal/pycore_global_objects.h
index 5a3fb132c7..442f851627 100644
--- a/Include/internal/pycore_global_objects.h
+++ b/Include/internal/pycore_global_objects.h
@@ -8,6 +8,7 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif
+#include "pycore_hashtable.h" // _Py_hashtable_t
#include "pycore_gc.h" // PyGC_Head
#include "pycore_global_strings.h" // struct _Py_global_strings
#include "pycore_hamt.h" // PyHamtNode_Bitmap
@@ -28,6 +29,11 @@ extern "C" {
#define _Py_SINGLETON(NAME) \
_Py_GLOBAL_OBJECT(singletons.NAME)
+struct _Py_cached_objects {
+ // XXX We could statically allocate the hashtable.
+ _Py_hashtable_t *interned_strings;
+};
+
struct _Py_static_objects {
struct {
/* Small integers are preallocated in this array so that they
diff --git a/Include/internal/pycore_hashtable.h b/Include/internal/pycore_hashtable.h
index 6501ab14d2..f57978a8d6 100644
--- a/Include/internal/pycore_hashtable.h
+++ b/Include/internal/pycore_hashtable.h
@@ -106,6 +106,7 @@ PyAPI_FUNC(int) _Py_hashtable_foreach(
void *user_data);
PyAPI_FUNC(size_t) _Py_hashtable_size(const _Py_hashtable_t *ht);
+PyAPI_FUNC(size_t) _Py_hashtable_len(const _Py_hashtable_t *ht);
/* Add a new entry to the hash. The key must not be present in the hash table.
Return 0 on success, -1 on memory error. */
diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h
index 5ed97e9715..a82a8ff0e0 100644
--- a/Include/internal/pycore_runtime.h
+++ b/Include/internal/pycore_runtime.h
@@ -161,6 +161,7 @@ typedef struct pyruntimestate {
/* All the objects that are shared by the runtime's interpreters. */
struct _Py_static_objects static_objects;
+ struct _Py_cached_objects cached_objects;
/* The following fields are here to avoid allocation during init.
The data is exposed through _PyRuntimeState pointer fields.
diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h
index 7aace9f861..660d7d7e23 100644
--- a/Include/internal/pycore_runtime_init.h
+++ b/Include/internal/pycore_runtime_init.h
@@ -164,6 +164,7 @@ extern PyTypeObject _PyExc_MemoryError;
.kind = 1, \
.compact = 1, \
.ascii = (ASCII), \
+ .statically_allocated = 1, \
}, \
}
#define _PyASCIIObject_INIT(LITERAL) \
diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
index c3e9f9c406..132e370c76 100644
--- a/Lib/test/test_sys.py
+++ b/Lib/test/test_sys.py
@@ -4,6 +4,7 @@
import locale
import operator
import os
+import random
import struct
import subprocess
import sys
@@ -18,6 +19,17 @@
import unittest
import warnings
+try:
+ from test.support import interpreters
+except ImportError:
+ interpreters = None
+
+
+def requires_subinterpreters(func):
+ deco = unittest.skipIf(interpreters is None,
+ 'Test requires subinterpreters')
+ return deco(func)
+
# count the number of test runs, used to create unique
# strings to intern in test_intern()
@@ -704,6 +716,35 @@ def __hash__(self):
self.assertRaises(TypeError, sys.intern, S("abc"))
+ @requires_subinterpreters
+ def test_subinterp_intern_dynamically_allocated(self):
+ s = "never interned before" + str(random.randrange(0, 10**9))
+ t = sys.intern(s)
+ self.assertIs(t, s)
+
+ interp = interpreters.create()
+ interp.run(textwrap.dedent(f'''
+ import sys
+ t = sys.intern({s!r})
+ assert id(t) != {id(s)}, (id(t), {id(s)})
+ assert id(t) != {id(t)}, (id(t), {id(t)})
+ '''))
+
+ @requires_subinterpreters
+ def test_subinterp_intern_statically_allocated(self):
+ # See Tools/build/generate_global_objects.py for the list
+ # of strings that are always statically allocated.
+ s = '__init__'
+ t = sys.intern(s)
+
+ print('------------------------')
+ interp = interpreters.create()
+ interp.run(textwrap.dedent(f'''
+ import sys
+ t = sys.intern({s!r})
+ assert id(t) == {id(t)}, (id(t), {id(t)})
+ '''))
+
def test_sys_flags(self):
self.assertTrue(sys.flags)
attrs = ("debug",
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-07-25-15-29-26.gh-issue-106931.kKU1le.rst b/Misc/NEWS.d/next/Core and Builtins/2023-07-25-15-29-26.gh-issue-106931.kKU1le.rst
new file mode 100644
index 0000000000..e0def5331b
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-07-25-15-29-26.gh-issue-106931.kKU1le.rst
@@ -0,0 +1,3 @@
+Statically allocated string objects are now interned globally instead of
+per-interpreter. This fixes a situation where such a string would only be
+interned in a single interpreter. Normal string objects are unaffected.
diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c
index 26aa13913d..ec82eda9ac 100644
--- a/Objects/unicodeobject.c
+++ b/Objects/unicodeobject.c
@@ -235,15 +235,54 @@ static inline PyObject *get_interned_dict(PyInterpreterState *interp)
return _Py_INTERP_CACHED_OBJECT(interp, interned_strings);
}
+#define INTERNED_STRINGS _PyRuntime.cached_objects.interned_strings
+
Py_ssize_t
_PyUnicode_InternedSize(void)
{
- return PyObject_Length(get_interned_dict(_PyInterpreterState_GET()));
+ PyObject *dict = get_interned_dict(_PyInterpreterState_GET());
+ return _Py_hashtable_len(INTERNED_STRINGS) + PyDict_GET_SIZE(dict);
+}
+
+static Py_hash_t unicode_hash(PyObject *);
+static int unicode_compare_eq(PyObject *, PyObject *);
+
+static Py_uhash_t
+hashtable_unicode_hash(const void *key)
+{
+ return unicode_hash((PyObject *)key);
+}
+
+static int
+hashtable_unicode_compare(const void *key1, const void *key2)
+{
+ PyObject *obj1 = (PyObject *)key1;
+ PyObject *obj2 = (PyObject *)key2;
+ if (obj1 != NULL && obj2 != NULL) {
+ return unicode_compare_eq(obj1, obj2);
+ }
+ else {
+ return obj1 == obj2;
+ }
}
static int
init_interned_dict(PyInterpreterState *interp)
{
+ if (_Py_IsMainInterpreter(interp)) {
+ assert(INTERNED_STRINGS == NULL);
+ _Py_hashtable_allocator_t hashtable_alloc = {PyMem_RawMalloc, PyMem_RawFree};
+ INTERNED_STRINGS = _Py_hashtable_new_full(
+ hashtable_unicode_hash,
+ hashtable_unicode_compare,
+ NULL,
+ NULL,
+ &hashtable_alloc
+ );
+ if (INTERNED_STRINGS == NULL) {
+ return -1;
+ }
+ }
assert(get_interned_dict(interp) == NULL);
PyObject *interned = interned = PyDict_New();
if (interned == NULL) {
@@ -262,6 +301,10 @@ clear_interned_dict(PyInterpreterState *interp)
Py_DECREF(interned);
_Py_INTERP_CACHED_OBJECT(interp, interned_strings) = NULL;
}
+ if (_Py_IsMainInterpreter(interp) && INTERNED_STRINGS != NULL) {
+ _Py_hashtable_destroy(INTERNED_STRINGS);
+ INTERNED_STRINGS = NULL;
+ }
}
#define _Py_RETURN_UNICODE_EMPTY() \
@@ -1222,6 +1265,7 @@ PyUnicode_New(Py_ssize_t size, Py_UCS4 maxchar)
_PyUnicode_STATE(unicode).kind = kind;
_PyUnicode_STATE(unicode).compact = 1;
_PyUnicode_STATE(unicode).ascii = is_ascii;
+ _PyUnicode_STATE(unicode).statically_allocated = 0;
if (is_ascii) {
((char*)data)[size] = 0;
}
@@ -1552,7 +1596,9 @@ unicode_dealloc(PyObject *unicode)
* we accidentally decref an immortal string out of existence. Since
* the string is an immortal object, just re-set the reference count.
*/
- if (PyUnicode_CHECK_INTERNED(unicode)) {
+ if (PyUnicode_CHECK_INTERNED(unicode)
+ || _PyUnicode_STATE(unicode).statically_allocated)
+ {
_Py_SetImmortal(unicode);
return;
}
@@ -14502,6 +14548,7 @@ unicode_subtype_new(PyTypeObject *type, PyObject *unicode)
_PyUnicode_STATE(self).kind = kind;
_PyUnicode_STATE(self).compact = 0;
_PyUnicode_STATE(self).ascii = _PyUnicode_STATE(unicode).ascii;
+ _PyUnicode_STATE(self).statically_allocated = 0;
_PyUnicode_UTF8_LENGTH(self) = 0;
_PyUnicode_UTF8(self) = NULL;
_PyUnicode_DATA_ANY(self) = NULL;
@@ -14725,6 +14772,23 @@ _PyUnicode_InternInPlace(PyInterpreterState *interp, PyObject **p)
return;
}
+ /* Look in the global cache first. */
+ PyObject *r = (PyObject *)_Py_hashtable_get(INTERNED_STRINGS, s);
+ if (r != NULL && r != s) {
+ Py_SETREF(*p, Py_NewRef(r));
+ return;
+ }
+
+ /* Handle statically allocated strings. */
+ if (_PyUnicode_STATE(s).statically_allocated) {
+ assert(_Py_IsImmortal(s));
+ if (_Py_hashtable_set(INTERNED_STRINGS, s, s) == 0) {
+ _PyUnicode_STATE(*p).interned = SSTATE_INTERNED_IMMORTAL_STATIC;
+ }
+ return;
+ }
+
+ /* Look in the per-interpreter cache. */
PyObject *interned = get_interned_dict(interp);
assert(interned != NULL);
@@ -14740,9 +14804,11 @@ _PyUnicode_InternInPlace(PyInterpreterState *interp, PyObject **p)
}
if (_Py_IsImmortal(s)) {
+ // XXX Restrict this to the main interpreter?
_PyUnicode_STATE(*p).interned = SSTATE_INTERNED_IMMORTAL_STATIC;
- return;
+ return;
}
+
#ifdef Py_REF_DEBUG
/* The reference count value excluding the 2 references from the
interned dictionary should be excluded from the RefTotal. The
diff --git a/Python/hashtable.c b/Python/hashtable.c
index 09501de199..9f005c7ad9 100644
--- a/Python/hashtable.c
+++ b/Python/hashtable.c
@@ -128,6 +128,13 @@ _Py_hashtable_size(const _Py_hashtable_t *ht)
}
+size_t
+_Py_hashtable_len(const _Py_hashtable_t *ht)
+{
+ return ht->nentries;
+}
+
+
_Py_hashtable_entry_t *
_Py_hashtable_get_entry_generic(_Py_hashtable_t *ht, const void *key)
{
diff --git a/Tools/build/deepfreeze.py b/Tools/build/deepfreeze.py
index b084d3e457..a11fe6a628 100644
--- a/Tools/build/deepfreeze.py
+++ b/Tools/build/deepfreeze.py
@@ -208,6 +208,7 @@ def generate_unicode(self, name: str, s: str) -> str:
self.write(".kind = 1,")
self.write(".compact = 1,")
self.write(".ascii = 1,")
+ self.write(".statically_allocated = 1,")
self.write(f"._data = {make_string_literal(s.encode('ascii'))},")
return f"& {name}._ascii.ob_base"
else:
@@ -220,6 +221,7 @@ def generate_unicode(self, name: str, s: str) -> str:
self.write(f".kind = {kind},")
self.write(".compact = 1,")
self.write(".ascii = 0,")
+ self.write(".statically_allocated = 1,")
utf8 = s.encode('utf-8')
self.write(f'.utf8 = {make_string_literal(utf8)},')
self.write(f'.utf8_length = {len(utf8)},')

View file

@ -17,7 +17,7 @@ URL: https://www.python.org/
#global prerel ...
%global upstream_version %{general_version}%{?prerel}
Version: %{general_version}%{?prerel:~%{prerel}}
Release: 2%{?dist}
Release: 3%{?dist}
License: Python-2.0.1
@ -364,6 +364,27 @@ Patch251: 00251-change-user-install-location.patch
# https://github.com/GrahamDumpleton/mod_wsgi/issues/730
Patch371: 00371-revert-bpo-1596321-fix-threading-_shutdown-for-the-main-thread-gh-28549-gh-28589.patch
# 00411 # b39f80246f9ad02d636c081929b7fffcd2de2f34
# gh-106931: Intern Statically Allocated Strings Globally
#
# We tried this before with a dict and for all interned strings.
# That ran into problems due to interpreter isolation.
# However, exclusively using a per-interpreter cache caused some inconsistency
# that can eliminate the benefit of interning.
# Here we circle back to using a global cache,
# but only for statically allocated strings.
# We also use a more-basic _Py_hashtable_t for that global cache instead of a dict.
#
# Ideally we would only have the global cache, but the optional isolation
# of each interpreter's allocator means that a non-static string object
# must not outlive its interpreter.
# Thus we would have to store a copy of each such interned string
# in the global cache, tied to the main interpreter.
#
# (cherry-picked from commit b72947a8d26915156323ccfd04d273199ecb870c)
# (cherry-picked from commit 3e6b32875207c39325ed8eded5819cfa5dcebc82)
Patch411: 00411-gh-106931-intern-statically-allocated-strings-globally.patch
# (New patches go here ^^^)
#
# When adding new patches to "python" and "python3" in Fedora, EL, etc.,
@ -1666,6 +1687,10 @@ CheckPython optimized
# ======================================================
%changelog
* Tue Dec 05 2023 Miro Hrončok <mhroncok@redhat.com> - 3.12.0-3
- Fix dataclasses in subinterpreters
- Fixes: rhzb#2252567
* Thu Oct 05 2023 Yaakov Selkowitz <yselkowi@redhat.com> - 3.12.0-2
- Use bundled libb2 in RHEL builds