Skip to content

Commit 18ea22a

Browse files
miss-islingtonericsnowcurrently
miss-islington
and
ericsnowcurrently
authored
[3.13] gh-120161: Fix a Crash in the _datetime Module (gh-120518)
In gh-120009 I used an atexit hook to finalize the _datetime module's static types at interpreter shutdown. However, atexit hooks are executed very early in finalization, which is a problem in the few cases where a subclass of one of those static types is still alive until the final GC collection. The static builtin types don't have this probably because they are finalized toward the end, after the final GC collection. To avoid the problem for _datetime, I have applied a similar approach here. Also, credit goes to @mgorny and @neonene for the new tests. FYI, I would have liked to take a slightly cleaner approach with managed static types, but wanted to get a smaller fix in first for the sake of backporting. I'll circle back to the cleaner approach with a future change on the main branch. (cherry picked from commit b2e71ff, AKA gh-120182) Co-authored-by: Eric Snow <[email protected]>
1 parent da40fa3 commit 18ea22a

File tree

6 files changed

+133
-71
lines changed

6 files changed

+133
-71
lines changed

‎Include/internal/pycore_typeobject.h‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,25 @@ extern "C" {
1717
#define _Py_TYPE_BASE_VERSION_TAG (2<<16)
1818
#define _Py_MAX_GLOBAL_TYPE_VERSION_TAG (_Py_TYPE_BASE_VERSION_TAG - 1)
1919

20+
/* For now we hard-code this to a value for which we are confident
21+
all the static builtin types will fit (for all builds). */
22+
#define _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES 200
23+
#define _Py_MAX_MANAGED_STATIC_EXT_TYPES 10
24+
#define _Py_MAX_MANAGED_STATIC_TYPES \
25+
(_Py_MAX_MANAGED_STATIC_BUILTIN_TYPES + _Py_MAX_MANAGED_STATIC_EXT_TYPES)
26+
2027
struct _types_runtime_state {
2128
/* Used to set PyTypeObject.tp_version_tag for core static types. */
2229
// bpo-42745: next_version_tag remains shared by all interpreters
2330
// because of static types.
2431
unsigned int next_version_tag;
32+
33+
struct {
34+
struct {
35+
PyTypeObject *type;
36+
int64_t interp_count;
37+
} types[_Py_MAX_MANAGED_STATIC_TYPES];
38+
} managed_static;
2539
};
2640

2741

@@ -42,11 +56,6 @@ struct type_cache {
4256
struct type_cache_entry hashtable[1 << MCACHE_SIZE_EXP];
4357
};
4458

45-
/* For now we hard-code this to a value for which we are confident
46-
all the static builtin types will fit (for all builds). */
47-
#define _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES 200
48-
#define _Py_MAX_MANAGED_STATIC_EXT_TYPES 10
49-
5059
typedef struct {
5160
PyTypeObject *type;
5261
int isbuiltin;
@@ -125,6 +134,7 @@ struct types_state {
125134

126135
extern PyStatus _PyTypes_InitTypes(PyInterpreterState *);
127136
extern void _PyTypes_FiniTypes(PyInterpreterState *);
137+
extern void _PyTypes_FiniExtTypes(PyInterpreterState *interp);
128138
extern void _PyTypes_Fini(PyInterpreterState *);
129139
extern void _PyTypes_AfterFork(void);
130140

@@ -163,10 +173,6 @@ extern managed_static_type_state * _PyStaticType_GetState(
163173
PyAPI_FUNC(int) _PyStaticType_InitForExtension(
164174
PyInterpreterState *interp,
165175
PyTypeObject *self);
166-
PyAPI_FUNC(void) _PyStaticType_FiniForExtension(
167-
PyInterpreterState *interp,
168-
PyTypeObject *self,
169-
int final);
170176

171177

172178
/* Like PyType_GetModuleState, but skips verification

‎Lib/test/datetimetester.py‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
from test import support
2525
from test.support import is_resource_enabled, ALWAYS_EQ, LARGEST, SMALLEST
26-
from test.support import warnings_helper
26+
from test.support import script_helper, warnings_helper
2727

2828
import datetime as datetime_module
2929
from datetime import MINYEAR, MAXYEAR
@@ -6840,6 +6840,48 @@ def run(type_checker, obj):
68406840
self.assertEqual(ret, 0)
68416841

68426842

6843+
class ExtensionModuleTests(unittest.TestCase):
6844+
6845+
def setUp(self):
6846+
if self.__class__.__name__.endswith('Pure'):
6847+
self.skipTest('Not relevant in pure Python')
6848+
6849+
@support.cpython_only
6850+
def test_gh_120161(self):
6851+
with self.subTest('simple'):
6852+
script = textwrap.dedent("""
6853+
import datetime
6854+
from _ast import Tuple
6855+
f = lambda: None
6856+
Tuple.dims = property(f, f)
6857+
6858+
class tzutc(datetime.tzinfo):
6859+
pass
6860+
""")
6861+
script_helper.assert_python_ok('-c', script)
6862+
6863+
with self.subTest('complex'):
6864+
script = textwrap.dedent("""
6865+
import asyncio
6866+
import datetime
6867+
from typing import Type
6868+
6869+
class tzutc(datetime.tzinfo):
6870+
pass
6871+
_EPOCHTZ = datetime.datetime(1970, 1, 1, tzinfo=tzutc())
6872+
6873+
class FakeDateMeta(type):
6874+
def __instancecheck__(self, obj):
6875+
return True
6876+
class FakeDate(datetime.date, metaclass=FakeDateMeta):
6877+
pass
6878+
def pickle_fake_date(datetime_) -> Type[FakeDate]:
6879+
# A pickle function for FakeDate
6880+
return FakeDate
6881+
""")
6882+
script_helper.assert_python_ok('-c', script)
6883+
6884+
68436885
def load_tests(loader, standard_tests, pattern):
68446886
standard_tests.addTest(ZoneInfoCompleteTest())
68456887
return standard_tests

‎Misc/NEWS.d/next/Library/2024-06-06-17-24-43.gh-issue-120161.DahNXV.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:mod:`datetime` no longer crashes in certain complex reference cycle
2+
situations.

‎Modules/_datetimemodule.c‎

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -7126,37 +7126,6 @@ clear_state(datetime_state *st)
71267126
}
71277127

71287128

7129-
/* ---------------------------------------------------------------------------
7130-
* Global module state.
7131-
*/
7132-
7133-
// If we make _PyStaticType_*ForExtension() public
7134-
// then all this should be managed by the runtime.
7135-
7136-
static struct {
7137-
PyMutex mutex;
7138-
int64_t interp_count;
7139-
} _globals = {0};
7140-
7141-
static void
7142-
callback_for_interp_exit(void *Py_UNUSED(data))
7143-
{
7144-
PyInterpreterState *interp = PyInterpreterState_Get();
7145-
7146-
assert(_globals.interp_count > 0);
7147-
PyMutex_Lock(&_globals.mutex);
7148-
_globals.interp_count -= 1;
7149-
int final = !_globals.interp_count;
7150-
PyMutex_Unlock(&_globals.mutex);
7151-
7152-
/* They must be done in reverse order so subclasses are finalized
7153-
* before base classes. */
7154-
for (size_t i = Py_ARRAY_LENGTH(capi_types); i > 0; i--) {
7155-
PyTypeObject *type = capi_types[i-1];
7156-
_PyStaticType_FiniForExtension(interp, type, final);
7157-
}
7158-
}
7159-
71607129
static int
71617130
init_static_types(PyInterpreterState *interp, int reloading)
71627131
{
@@ -7179,19 +7148,6 @@ init_static_types(PyInterpreterState *interp, int reloading)
71797148
}
71807149
}
71817150

7182-
PyMutex_Lock(&_globals.mutex);
7183-
assert(_globals.interp_count >= 0);
7184-
_globals.interp_count += 1;
7185-
PyMutex_Unlock(&_globals.mutex);
7186-
7187-
/* It could make sense to add a separate callback
7188-
* for each of the types. However, for now we can take the simpler
7189-
* approach of a single callback. */
7190-
if (PyUnstable_AtExit(interp, callback_for_interp_exit, NULL) < 0) {
7191-
callback_for_interp_exit(NULL);
7192-
return -1;
7193-
}
7194-
71957151
return 0;
71967152
}
71977153

@@ -7376,8 +7332,8 @@ module_clear(PyObject *mod)
73767332
PyInterpreterState *interp = PyInterpreterState_Get();
73777333
clear_current_module(interp, mod);
73787334

7379-
// We take care of the static types via an interpreter atexit hook.
7380-
// See callback_for_interp_exit() above.
7335+
// The runtime takes care of the static types for us.
7336+
// See _PyTypes_FiniExtTypes()..
73817337

73827338
return 0;
73837339
}

‎Objects/typeobject.c‎

Lines changed: 70 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -159,18 +159,28 @@ managed_static_type_index_clear(PyTypeObject *self)
159159
self->tp_subclasses = NULL;
160160
}
161161

162-
static inline managed_static_type_state *
163-
static_builtin_state_get(PyInterpreterState *interp, PyTypeObject *self)
162+
static PyTypeObject *
163+
static_ext_type_lookup(PyInterpreterState *interp, size_t index,
164+
int64_t *p_interp_count)
164165
{
165-
return &(interp->types.builtins.initialized[
166-
managed_static_type_index_get(self)]);
167-
}
166+
assert(interp->runtime == &_PyRuntime);
167+
assert(index < _Py_MAX_MANAGED_STATIC_EXT_TYPES);
168168

169-
static inline managed_static_type_state *
170-
static_ext_type_state_get(PyInterpreterState *interp, PyTypeObject *self)
171-
{
172-
return &(interp->types.for_extensions.initialized[
173-
managed_static_type_index_get(self)]);
169+
size_t full_index = index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES;
170+
int64_t interp_count =
171+
_PyRuntime.types.managed_static.types[full_index].interp_count;
172+
assert((interp_count == 0) ==
173+
(_PyRuntime.types.managed_static.types[full_index].type == NULL));
174+
*p_interp_count = interp_count;
175+
176+
PyTypeObject *type = interp->types.for_extensions.initialized[index].type;
177+
if (type == NULL) {
178+
return NULL;
179+
}
180+
assert(!interp->types.for_extensions.initialized[index].isbuiltin);
181+
assert(type == _PyRuntime.types.managed_static.types[full_index].type);
182+
assert(managed_static_type_index_is_set(type));
183+
return type;
174184
}
175185

176186
static managed_static_type_state *
@@ -202,6 +212,8 @@ static void
202212
managed_static_type_state_init(PyInterpreterState *interp, PyTypeObject *self,
203213
int isbuiltin, int initial)
204214
{
215+
assert(interp->runtime == &_PyRuntime);
216+
205217
size_t index;
206218
if (initial) {
207219
assert(!managed_static_type_index_is_set(self));
@@ -228,6 +240,21 @@ managed_static_type_state_init(PyInterpreterState *interp, PyTypeObject *self,
228240
assert(index < _Py_MAX_MANAGED_STATIC_EXT_TYPES);
229241
}
230242
}
243+
size_t full_index = isbuiltin
244+
? index
245+
: index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES;
246+
247+
assert((initial == 1) ==
248+
(_PyRuntime.types.managed_static.types[full_index].interp_count == 0));
249+
_PyRuntime.types.managed_static.types[full_index].interp_count += 1;
250+
251+
if (initial) {
252+
assert(_PyRuntime.types.managed_static.types[full_index].type == NULL);
253+
_PyRuntime.types.managed_static.types[full_index].type = self;
254+
}
255+
else {
256+
assert(_PyRuntime.types.managed_static.types[full_index].type == self);
257+
}
231258

232259
managed_static_type_state *state = isbuiltin
233260
? &(interp->types.builtins.initialized[index])
@@ -256,15 +283,28 @@ static void
256283
managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self,
257284
int isbuiltin, int final)
258285
{
286+
size_t index = managed_static_type_index_get(self);
287+
size_t full_index = isbuiltin
288+
? index
289+
: index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES;
290+
259291
managed_static_type_state *state = isbuiltin
260-
? static_builtin_state_get(interp, self)
261-
: static_ext_type_state_get(interp, self);
292+
? &(interp->types.builtins.initialized[index])
293+
: &(interp->types.for_extensions.initialized[index]);
294+
assert(state != NULL);
295+
296+
assert(_PyRuntime.types.managed_static.types[full_index].interp_count > 0);
297+
assert(_PyRuntime.types.managed_static.types[full_index].type == state->type);
262298

263299
assert(state->type != NULL);
264300
state->type = NULL;
265301
assert(state->tp_weaklist == NULL); // It was already cleared out.
266302

303+
_PyRuntime.types.managed_static.types[full_index].interp_count -= 1;
267304
if (final) {
305+
assert(!_PyRuntime.types.managed_static.types[full_index].interp_count);
306+
_PyRuntime.types.managed_static.types[full_index].type = NULL;
307+
268308
managed_static_type_index_clear(self);
269309
}
270310

@@ -840,8 +880,12 @@ _PyTypes_Fini(PyInterpreterState *interp)
840880
struct type_cache *cache = &interp->types.type_cache;
841881
type_cache_clear(cache, NULL);
842882

883+
// All the managed static types should have been finalized already.
884+
assert(interp->types.for_extensions.num_initialized == 0);
885+
for (size_t i = 0; i < _Py_MAX_MANAGED_STATIC_EXT_TYPES; i++) {
886+
assert(interp->types.for_extensions.initialized[i].type == NULL);
887+
}
843888
assert(interp->types.builtins.num_initialized == 0);
844-
// All the static builtin types should have been finalized already.
845889
for (size_t i = 0; i < _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES; i++) {
846890
assert(interp->types.builtins.initialized[i].type == NULL);
847891
}
@@ -5674,9 +5718,20 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type,
56745718
}
56755719

56765720
void
5677-
_PyStaticType_FiniForExtension(PyInterpreterState *interp, PyTypeObject *type, int final)
5721+
_PyTypes_FiniExtTypes(PyInterpreterState *interp)
56785722
{
5679-
fini_static_type(interp, type, 0, final);
5723+
for (size_t i = _Py_MAX_MANAGED_STATIC_EXT_TYPES; i > 0; i--) {
5724+
if (interp->types.for_extensions.num_initialized == 0) {
5725+
break;
5726+
}
5727+
int64_t count = 0;
5728+
PyTypeObject *type = static_ext_type_lookup(interp, i-1, &count);
5729+
if (type == NULL) {
5730+
continue;
5731+
}
5732+
int final = (count == 1);
5733+
fini_static_type(interp, type, 0, final);
5734+
}
56805735
}
56815736

56825737
void

‎Python/pylifecycle.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1818,6 +1818,7 @@ flush_std_files(void)
18181818
static void
18191819
finalize_interp_types(PyInterpreterState *interp)
18201820
{
1821+
_PyTypes_FiniExtTypes(interp);
18211822
_PyUnicode_FiniTypes(interp);
18221823
_PySys_FiniTypes(interp);
18231824
_PyXI_FiniTypes(interp);

0 commit comments

Comments
 (0)