Repository navigation
Tracking un-needed nested locks #159558
Description
Spin off issue from #159486 continuing the automated findings on un-needed locks and estimate on performance gain. Tracked here are many easy fixable ones which should be able to tackled all or mostly at once.
| # | Method (lock holder) | Reference | Re-enters through | Per op | main → fixed | Why the lock is taken twice |
|---|---|---|---|---|---|---|
| 1 | set >= s, set > s, set.issuperset(s) |
Objects/setobject.c:2463 |
set_issubset clinic wrapper (CS2 on both sets) |
+1 (CS2) | 34 → 21 ns (1.6–1.9x) | Argument Clinic generates a wrapper that takes the lock, then calls *_impl. issuperset is written as "issubset with the arguments swapped" and calls the wrapper rather than the impl, so the pair of sets it already holds is locked a second time. |
| 2 | set.discard(x), set.remove(x) |
setobject.c:2692, :2652 → set_discard_entry :584 → set_lookkey CS :423 |
set_lookkey |
+1 | 34 → 23 ns (1.45x) | set_lookkey is shared by callers that run without a lock (in, C API) and by ones that already hold it, so it always locks mutable sets itself. The discard/remove impls hold the lock from their wrapper and go through it anyway; there was no lock-held variant. |
| 3 | ctypes Simple.value setter (c_int().value = 1) |
Modules/_ctypes/_ctypes.c:5406 |
KeepRef CS(target) :2982, then PyCData_GetContainer CS(root) :2869 |
+1 scalar, +2 object | c_int 35 → 22 ns (1.55x); c_char_p 39 → 22 ns (1.7x) |
KeepRef was made self-locking when ctypes got free-threading support, so it is safe from any caller; the setter's clinic wrapper also locks self, giving two locks on the same object. For object-keeping types KeepRef then calls PyCData_GetContainer, which locks the root object, and for a top-level object the root is self: a third lock. The scalar case still paid once, because the "nothing to keep" early return sits inside the locked region. |
| 4 | ctypes Pointer.contents setter |
_ctypes.c:5705, :5713, :5722 under CS2 :5725 |
two KeepRef calls, plus GetKeepedObjects(value) → PyCData_GetContainer(value) |
+2 (+1) | 86 → 58 ns (1.5x) | The setter takes a two-object critical section on the pointer and the new target, then calls the same self-locking helpers as #3 on both of them: KeepRef on the pointer, and the container lookup on the target. |
| 5 | ctypes field and item stores via PyCData_set |
_ctypes.c:3537; asserts dst locked at :3531; callers PyCField_set, :5168, :5596 |
KeepRef(mem, index, result) |
+1 | S.i = 1 38 → 24 ns (1.55x); S.p = b"" 69 → 54 ns (1.27x) |
PyCData_set already requires its caller to hold the destination's lock (it asserts it), but finishes by calling the self-locking KeepRef on that same destination. The contract was there; the last call just didn't use it. |
Activity
pythongh-159558: Add NEWS entries for the set and ctypes critical-sec…
pythongh-159558: Combine the NEWS entries
pythongh-159558: Drop a redundant comment