Skip to content

gh-112075: Add critical sections for most dict APIs #114508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DinoV merged 5 commits into from
Feb 6, 2024
Merged

gh-112075: Add critical sections for most dict APIs #114508

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
455461b
Add macro to assert critical section is held
DinoV Jan 10, 2024
a0d1e27
Critical sections
DinoV Jan 18, 2024
584f4cb
Most dict operations should use locks
DinoV Jan 19, 2024
aa80085
Lock someplaces outside of dicts & review feedback
DinoV Jan 23, 2024
ab5d224
Fix formatting
DinoV Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Fix formatting
  • Loading branch information
@DinoV
DinoV committed Feb 6, 2024
commit ab5d224fabf25400f3564638a71f3976dfa855f4
15 changes: 12 additions & 3 deletions Include/internal/pycore_critical_section.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,18 @@ extern "C" {
// Asserts that the mutex for the given object is locked. The mutex must
// be held by the top-most critical section otherwise there's the
// possibility that the mutex would be swalled out in some code paths.
#define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op) \
_Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&_PyObject_CAST(op)->ob_mutex)
#ifdef Py_DEBUG

#define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op) \
if (Py_REFCNT(op) != 1) { \
_Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&_PyObject_CAST(op)->ob_mutex); \
}

#else /* Py_DEBUG */

#define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op)

#endif /* Py_DEBUG */

#else /* !Py_GIL_DISABLED */
// The critical section APIs are no-ops with the GIL.
Expand Down Expand Up @@ -263,7 +271,8 @@ _PyCriticalSection_AssertHeld(PyMutex *mutex) {
if (prev & _Py_CRITICAL_SECTION_TWO_MUTEXES) {
_PyCriticalSection2 *cs = (_PyCriticalSection2 *)(prev & ~_Py_CRITICAL_SECTION_MASK);
assert(cs != NULL && (cs->base.mutex == mutex || cs->mutex2 == mutex));
} else {
}
else {
_PyCriticalSection *cs = (_PyCriticalSection *)(tstate->critical_section & ~_Py_CRITICAL_SECTION_MASK);
assert(cs != NULL && cs->mutex == mutex);
}
Expand Down
12 changes: 5 additions & 7 deletions Objects/dictobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,7 @@ it's USABLE_FRACTION (currently two-thirds) full.
static inline void
ASSERT_DICT_LOCKED(PyObject *op)
{
if (Py_REFCNT(op) != 1) {
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op);
}
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op);
}
#define ASSERT_DICT_LOCKED(op) ASSERT_DICT_LOCKED(_Py_CAST(PyObject*, op))

Expand Down Expand Up @@ -1988,7 +1986,8 @@ _PyDict_SetItem_KnownHash(PyObject *op, PyObject *key, PyObject *value,

if (mp->ma_keys == Py_EMPTY_KEYS) {
res = insert_to_emptydict(interp, mp, Py_NewRef(key), hash, Py_NewRef(value));
} else {
}
else {
/* insertdict() handles any resizing that might be necessary */
res = insertdict(interp, mp, Py_NewRef(key), hash, Py_NewRef(value));
}
Expand Down Expand Up @@ -2522,7 +2521,8 @@ _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)
d = (PyObject *)dict_dict_fromkeys(interp, mp, iterable, value);
Py_END_CRITICAL_SECTION2();
return d;
} else if (PyAnySet_CheckExact(iterable)) {
}
else if (PyAnySet_CheckExact(iterable)) {
PyDictObject *mp = (PyDictObject *)d;

Py_BEGIN_CRITICAL_SECTION2(d, iterable);
Expand Down Expand Up @@ -2875,7 +2875,6 @@ PyDict_Values(PyObject *dict)
Py_BEGIN_CRITICAL_SECTION(dict);
res = values_lock_held(dict);
Py_END_CRITICAL_SECTION();

return res;
Comment thread
DinoV marked this conversation as resolved.
}

Expand Down Expand Up @@ -4135,7 +4134,6 @@ PyDict_Contains(PyObject *op, PyObject *key)
Py_BEGIN_CRITICAL_SECTION(op);
res = contains_lock_held((PyDictObject *)op, key);
Py_END_CRITICAL_SECTION();

return res;
Comment thread
DinoV marked this conversation as resolved.
}

Expand Down