Skip to content

Commit 51a4b93

Browse files
committed
Review feedback
1 parent 3799cbe commit 51a4b93

File tree

3 files changed

+28
-25
lines changed

3 files changed

+28
-25
lines changed

Include/internal/pycore_critical_section.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,8 @@ _PyCriticalSection_SuspendAll(PyThreadState *tstate);
293293
#ifdef Py_GIL_DISABLED
294294

295295
static inline void
296-
_PyCriticalSection_AssertHeld(PyMutex *mutex) {
296+
_PyCriticalSection_AssertHeld(PyMutex *mutex)
297+
{
297298
#ifdef Py_DEBUG
298299
PyThreadState *tstate = _PyThreadState_GET();
299300
uintptr_t prev = tstate->critical_section;

Include/internal/pycore_typeobject.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ struct types_state {
7979
extern PyStatus _PyTypes_InitTypes(PyInterpreterState *);
8080
extern void _PyTypes_FiniTypes(PyInterpreterState *);
8181
extern void _PyTypes_Fini(PyInterpreterState *);
82-
void _PyTypes_AfterFork(void);
82+
extern void _PyTypes_AfterFork(void);
8383

8484
/* other API */
8585

Objects/typeobject.c

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -686,9 +686,15 @@ type_cache_clear(struct type_cache *cache, PyObject *value)
686686
{
687687
for (Py_ssize_t i = 0; i < (1 << MCACHE_SIZE_EXP); i++) {
688688
struct type_cache_entry *entry = &cache->hashtable[i];
689+
#ifdef Py_GIL_DISABLED
690+
_PySeqLock_LockWrite(&entry->sequence);
691+
#endif
689692
entry->version = 0;
690693
Py_XSETREF(entry->name, _Py_XNewRef(value));
691694
entry->value = NULL;
695+
#ifdef Py_GIL_DISABLED
696+
_PySeqLock_UnlockWrite(&entry->sequence);
697+
#endif
692698
}
693699
}
694700

@@ -4903,6 +4909,8 @@ update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int versio
49034909
entry->value = value; /* borrowed */
49044910
assert(_PyASCIIObject_CAST(name)->hash != -1);
49054911
OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name);
4912+
// We're releasing this under the lock for simplicity sake because it's always a
4913+
// exact unicode object or Py_None so it's safe to do so.
49064914
Py_SETREF(entry->name, Py_NewRef(name));
49074915
}
49084916

@@ -4933,15 +4941,17 @@ update_cache_gil_disabled(struct type_cache_entry *entry, PyObject *name,
49334941

49344942
#endif
49354943

4936-
void _PyTypes_AfterFork() {
4944+
void
4945+
_PyTypes_AfterFork()
4946+
{
49374947
#ifdef Py_GIL_DISABLED
49384948
struct type_cache *cache = get_type_cache();
4939-
for (int i = 0; i < 1 << MCACHE_SIZE_EXP; i++) {
4949+
for (Py_ssize_t i = 0; i < (1 << MCACHE_SIZE_EXP); i++) {
49404950
struct type_cache_entry *entry = &cache->hashtable[i];
49414951
if (_PySeqLock_AfterFork(&entry->sequence)) {
49424952
// Entry was in the process of updating while forking, clear it...
49434953
entry->value = NULL;
4944-
entry->name = NULL;
4954+
Py_SETREF(entry->name, Py_None);
49454955
entry->version = 0;
49464956
}
49474957
}
@@ -5005,6 +5015,7 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name)
50055015
if (MCACHE_CACHEABLE_NAME(name)) {
50065016
has_version = assign_version_tag(interp, type);
50075017
version = type->tp_version_tag;
5018+
assert(!has_version || _PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG));
50085019
}
50095020
END_TYPE_LOCK()
50105021

@@ -5030,8 +5041,7 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name)
50305041
#else
50315042
update_cache(entry, name, version, res);
50325043
#endif
5033-
assert(_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG));
5034-
}
5044+
}
50355045
return res;
50365046
}
50375047

@@ -9506,13 +9516,13 @@ slot_bf_getbuffer(PyObject *self, Py_buffer *buffer, int flags)
95069516
return -1;
95079517
}
95089518

9509-
static int
9510-
releasebuffer_maybe_call_super_unlocked(PyObject *self, Py_buffer *buffer, releasebufferproc *base_releasebuffer)
9519+
static releasebufferproc
9520+
releasebuffer_maybe_call_super_unlocked(PyObject *self, Py_buffer *buffer)
95119521
{
95129522
PyTypeObject *self_type = Py_TYPE(self);
95139523
PyObject *mro = lookup_tp_mro(self_type);
95149524
if (mro == NULL) {
9515-
return -1;
9525+
return NULL;
95169526
}
95179527

95189528
assert(PyTuple_Check(mro));
@@ -9526,9 +9536,8 @@ releasebuffer_maybe_call_super_unlocked(PyObject *self, Py_buffer *buffer, relea
95269536
}
95279537
i++; /* skip self_type */
95289538
if (i >= n)
9529-
return -1;
9539+
return NULL;
95309540

9531-
*base_releasebuffer = NULL;
95329541
for (; i < n; i++) {
95339542
PyObject *obj = PyTuple_GET_ITEM(mro, i);
95349543
if (!PyType_Check(obj)) {
@@ -9538,28 +9547,25 @@ releasebuffer_maybe_call_super_unlocked(PyObject *self, Py_buffer *buffer, relea
95389547
if (base_type->tp_as_buffer != NULL
95399548
&& base_type->tp_as_buffer->bf_releasebuffer != NULL
95409549
&& base_type->tp_as_buffer->bf_releasebuffer != slot_bf_releasebuffer) {
9541-
*base_releasebuffer = base_type->tp_as_buffer->bf_releasebuffer;
9542-
break;
9550+
return base_type->tp_as_buffer->bf_releasebuffer;
95439551
}
95449552
}
95459553

9546-
return 0;
9554+
return NULL;
95479555
}
95489556

9549-
static int
9557+
static void
95509558
releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer)
95519559
{
9552-
int res;
95539560
releasebufferproc base_releasebuffer;
95549561

95559562
BEGIN_TYPE_LOCK();
9556-
res = releasebuffer_maybe_call_super_unlocked(self, buffer, &base_releasebuffer);
9563+
base_releasebuffer = releasebuffer_maybe_call_super_unlocked(self, buffer);
95579564
END_TYPE_LOCK();
95589565

9559-
if (res == 0) {
9566+
if (base_releasebuffer != NULL) {
95609567
base_releasebuffer(self, buffer);
95619568
}
9562-
return res;
95639569
}
95649570

95659571
static void
@@ -9636,11 +9642,7 @@ static void
96369642
slot_bf_releasebuffer(PyObject *self, Py_buffer *buffer)
96379643
{
96389644
releasebuffer_call_python(self, buffer);
9639-
if (releasebuffer_maybe_call_super(self, buffer) < 0) {
9640-
if (PyErr_Occurred()) {
9641-
PyErr_WriteUnraisable(self);
9642-
}
9643-
}
9645+
releasebuffer_maybe_call_super(self, buffer);
96449646
}
96459647

96469648
static PyObject *

0 commit comments

Comments
 (0)