Skip to content

Commit 46061cb

Browse files
oprypincopybara-github
authored andcommitted
Prevent crashes when creating objects during interpreter shutdown
(The crashes happen only since Python 3.13) In particular: * Skip adding to the weak map if `Py_IsFinalizing()` is true. Calling `PyUpb_WeakMap_Add` may be caused by user code that creates protos inside `__del__`! * `PyUpb_DescriptorPool_Get` and subsequently `PyUpb_ObjCache_Get` may *also* be called if users cause `PyUpb_RepeatedContainer_GetOrCreateWrapper` to be called. So make it nullable and skip the assertion if currently shutting down. This follows a prior attempted fix d57d270 but expands the scope of it. PiperOrigin-RevId: 856589514
1 parent 12d5002 commit 46061cb

File tree

2 files changed

+49
-10
lines changed

2 files changed

+49
-10
lines changed

python/descriptor_pool.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "python/descriptor.h"
1313
#include "python/message.h"
1414
#include "python/protobuf.h"
15+
#include "python/python_api.h"
1516
#include "upb/base/upcast.h"
1617
#include "upb/message/compare.h"
1718
#include "upb/reflection/def.h"
@@ -75,7 +76,11 @@ static int PyUpb_DescriptorPool_Clear(PyUpb_DescriptorPool* self) {
7576

7677
PyObject* PyUpb_DescriptorPool_Get(const upb_DefPool* symtab) {
7778
PyObject* pool = PyUpb_ObjCache_Get(symtab);
78-
assert(pool);
79+
assert(pool
80+
#if PY_VERSION_HEX >= 0x030D0000 // >= 3.13
81+
|| Py_IsFinalizing()
82+
#endif
83+
);
7984
return pool;
8085
}
8186

python/protobuf.c

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -180,24 +180,58 @@ PyUpb_WeakMap* PyUpb_ObjCache_Instance(void) {
180180
return state->obj_cache;
181181
}
182182

183+
static PyUpb_WeakMap* PyUpb_ObjCache_MaybeInstance(void) {
184+
// During the shutdown sequence, our objects may need to be deallocated, or
185+
// there may even still be attempts to *construct* new ones (in user code).
186+
// At that point our state will be NULL, so all access to the cache has to be
187+
// no-op.
188+
PyUpb_ModuleState* state = PyUpb_ModuleState_MaybeGet();
189+
if (!state) {
190+
return NULL;
191+
}
192+
return state->obj_cache;
193+
}
194+
183195
void PyUpb_ObjCache_Add(const void* key, PyObject* py_obj) {
184-
PyUpb_WeakMap_Add(PyUpb_ObjCache_Instance(), key, py_obj);
196+
PyUpb_WeakMap* cache = PyUpb_ObjCache_MaybeInstance();
197+
if (!cache) {
198+
return;
199+
}
200+
PyUpb_WeakMap_Add(cache, key, py_obj);
185201
}
186202

187203
void PyUpb_ObjCache_Delete(const void* key) {
188-
PyUpb_ModuleState* state = PyUpb_ModuleState_MaybeGet();
189-
if (!state) {
190-
// During the shutdown sequence, our object's Dealloc() methods can be
191-
// called *after* our module Dealloc() method has been called. At that
192-
// point our state will be NULL and there is nothing to delete out of the
193-
// map.
204+
PyUpb_WeakMap* cache = PyUpb_ObjCache_MaybeInstance();
205+
if (!cache) {
194206
return;
195207
}
196-
PyUpb_WeakMap_Delete(state->obj_cache, key);
208+
PyUpb_WeakMap_Delete(cache, key);
197209
}
198210

199211
PyObject* PyUpb_ObjCache_Get(const void* key) {
200-
return PyUpb_WeakMap_Get(PyUpb_ObjCache_Instance(), key);
212+
PyUpb_WeakMap* cache = PyUpb_ObjCache_MaybeInstance();
213+
if (!cache) {
214+
// When the interpreter is finalizing, the state may not be available, so
215+
// we don't try to find existing objects in the object cache - in order to
216+
// prevent a crash. Unfortunately, doing this can lead to different
217+
// semantics in protobuf APIs during shutdown. For example:
218+
//
219+
// m = MyMessage()
220+
//
221+
// # Returns true most of the time, but false during shutdown.
222+
// assert(m.submsg is m.submsg)
223+
// assert(Py_IsFinalizing());
224+
//
225+
//
226+
// # Returns true most of the time, but false during shutdown.
227+
// assert(foo_pb2.DESCRIPTOR.message_types_by_name['Bar'] is
228+
// foo_pb2.DESCRIPTOR.message_types_by_name['Bar')
229+
#if PY_VERSION_HEX >= 0x030D0000 // >= 3.13
230+
assert(Py_IsFinalizing());
231+
#endif
232+
return NULL;
233+
}
234+
return PyUpb_WeakMap_Get(cache, key);
201235
}
202236

203237
// -----------------------------------------------------------------------------

0 commit comments

Comments
 (0)