FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-120161: Fix a Crash in the _datetime Module (gh-120182) · python/cpython@b2e71ff · GitHub

/ cpython Public

Commit b2e71ff

Browse files
ericsnowcurrently
authored
gh-120161: Fix a Crash in the _datetime Module (gh-120182)
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.
1 parent 05df063 commit b2e71ff

6 files changed

Lines changed: 133 additions & 71 deletions

File tree

‎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;
@@ -133,6 +142,7 @@ struct types_state {
133142

134143
extern PyStatus _PyTypes_InitTypes(PyInterpreterState *);
135144
extern void _PyTypes_FiniTypes(PyInterpreterState *);
145+
extern void _PyTypes_FiniExtTypes(PyInterpreterState *interp);
136146
extern void _PyTypes_Fini(PyInterpreterState *);
137147
extern void _PyTypes_AfterFork(void);
138148

@@ -171,10 +181,6 @@ extern managed_static_type_state * _PyStaticType_GetState(
171181
PyAPI_FUNC(int) _PyStaticType_InitForExtension(
172182
PyInterpreterState *interp,
173183
PyTypeObject *self);
174-
PyAPI_FUNC(void) _PyStaticType_FiniForExtension(
175-
PyInterpreterState *interp,
176-
PyTypeObject *self,
177-
int final);
178184

179185

180186
/* 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
@@ -6822,6 +6822,48 @@ def run(type_checker, obj):
68226822
self.assertEqual(ret, 0)
68236823

68246824

6825+
class ExtensionModuleTests(unittest.TestCase):
6826+
6827+
def setUp(self):
6828+
if self.__class__.__name__.endswith('Pure'):
6829+
self.skipTest('Not relevant in pure Python')
6830+
6831+
@support.cpython_only
6832+
def test_gh_120161(self):
6833+
with self.subTest('simple'):
6834+
script = textwrap.dedent("""
6835+
import datetime
6836+
from _ast import Tuple
6837+
f = lambda: None
6838+
Tuple.dims = property(f, f)
6839+
6840+
class tzutc(datetime.tzinfo):
6841+
pass
6842+
""")
6843+
script_helper.assert_python_ok('-c', script)
6844+
6845+
with self.subTest('complex'):
6846+
script = textwrap.dedent("""
6847+
import asyncio
6848+
import datetime
6849+
from typing import Type
6850+
6851+
class tzutc(datetime.tzinfo):
6852+
pass
6853+
_EPOCHTZ = datetime.datetime(1970, 1, 1, tzinfo=tzutc())
6854+
6855+
class FakeDateMeta(type):
6856+
def __instancecheck__(self, obj):
6857+
return True
6858+
class FakeDate(datetime.date, metaclass=FakeDateMeta):
6859+
pass
6860+
def pickle_fake_date(datetime_) -> Type[FakeDate]:
6861+
# A pickle function for FakeDate
6862+
return FakeDate
6863+
""")
6864+
script_helper.assert_python_ok('-c', script)
6865+
6866+
68256867
def load_tests(loader, standard_tests, pattern):
68266868
standard_tests.addTest(ZoneInfoCompleteTest())
68276869
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
@@ -7129,37 +7129,6 @@ clear_state(datetime_state *st)
71297129
}
71307130

71317131

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

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

@@ -7379,8 +7335,8 @@ module_clear(PyObject *mod)
73797335
PyInterpreterState *interp = PyInterpreterState_Get();
73807336
clear_current_module(interp, mod);
73817337

7382-
// We take care of the static types via an interpreter atexit hook.
7383-
// See callback_for_interp_exit() above.
7338+
// The runtime takes care of the static types for us.
7339+
// See _PyTypes_FiniExtTypes()..
73847340

73857341
return 0;
73867342
}

‎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
}
@@ -5834,9 +5878,20 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type,
58345878
}
58355879

58365880
void
5837-
_PyStaticType_FiniForExtension(PyInterpreterState *interp, PyTypeObject *type, int final)
5881+
_PyTypes_FiniExtTypes(PyInterpreterState *interp)
58385882
{
5839-
fini_static_type(interp, type, 0, final);
5883+
for (size_t i = _Py_MAX_MANAGED_STATIC_EXT_TYPES; i > 0; i--) {
5884+
if (interp->types.for_extensions.num_initialized == 0) {
5885+
break;
5886+
}
5887+
int64_t count = 0;
5888+
PyTypeObject *type = static_ext_type_lookup(interp, i-1, &count);
5889+
if (type == NULL) {
5890+
continue;
5891+
}
5892+
int final = (count == 1);
5893+
fini_static_type(interp, type, 0, final);
5894+
}
58405895
}
58415896

58425897
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)
Back | FazBrowse Home | New Git URL