Race in pickle.dumps/loads per-interpreter cache leaks references

[CharlieKerfoot]

Bug report

Bug description:

PyObject *dumps = state->pickle.dumps;
  if (dumps != NULL) {
      return dumps;
  }
  dumps = PyImport_ImportModuleAttrString("pickle", "dumps");
  if (dumps == NULL) return NULL;
  state->pickle.dumps = dumps;
  return dumps;

This is a check-then-act race. Two threads in the same interpreter can both run this code:

1. Thread A reads state->pickle.dumps → NULL
2. Thread B reads state->pickle.dumps → NULL
3. Thread A imports pickle.dumps, holds a strong reference R_A
4. Thread B imports pickle.dumps, holds a strong reference R_B
5. Thread A stores R_A into the slot
6. Thread B stores R_B into the slot, overwriting R_A

Now the slot owns R_B, and R_A is leaked forever.

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

• gh-149000: Fix race / leak in pickle XID per-interpreter cae #149002

[picnixz]

Do you have a reproducer for that? just for testing purposes.

[CharlieKerfoot]

Yea here is a reproducer script. When I ran it, 52/100 trials leaked references. Some trials lost the CAS race more than once (deltas of 3 and 4 observed). This was the build: CPython main @ fc829e88753858c8ac669594bf0093f44948c0f4, ./configure --disable-gil --with-pydebug, macOS arm64.

Reproducer

"""
Reproducer for the race in the per-interpreter pickle.{dumps,loads} cache
in Python/crossinterp.c (gh-149000).

1. Create one fresh subinterpreter (pickle XID cache empty).
2. Inside it, spawn N threads gated on a Barrier so they release together.
3. Each thread Queue.put()'s a non-shareable object, which forces the
 pickle XID fallback (_PyPickle_GetXIData -> _PyPickle_Dumps ->
 _get_pickle_dumps) and races on the cache slot.

sys.getrefcount(pickle.dumps) inside the subinterpreter, before vs. after:
* No race: delta == 1 (cache slot's reference only)
* Race hit: delta  > 1 (cache slot + one leaked ref per lost CAS)

The race is pretty much unobservable on the GIL build (the import path
doesn't release the GIL when `pickle` is already in sys.modules). Build
with `./configure --disable-gil` to surface it reliably. Increasing
N_THREADS or N_TRIALS increases the hit rate.
"""

from concurrent import interpreters


CHILD = r"""
import sys
import threading
import pickle
from concurrent import interpreters

N_THREADS = 256

class NonShareable:
  # Plain Python class instances are not cross-interpreter-shareable, so
  # transferring one through a Queue forces the pickle XID fallback.
  def __init__(self, n):
      self.n = n


q = interpreters.create_queue()
barrier = threading.Barrier(N_THREADS)
errors = []

baseline = sys.getrefcount(pickle.dumps)


def hammer():
  try:
      barrier.wait()
      q.put(NonShareable(0))
  except BaseException as e:
      errors.append(repr(e))


threads = [threading.Thread(target=hammer) for _ in range(N_THREADS)]
for t in threads:
  t.start()
for t in threads:
  t.join()

after = sys.getrefcount(pickle.dumps)
delta = after - baseline

# Drain the queue so held items don't confuse the count.
drained = 0
try:
  while True:
      q.get_nowait()
      drained += 1
except interpreters.QueueEmpty:
  pass

print(f"baseline={baseline} after={after} delta={delta} "
    f"drained={drained} errors={len(errors)}")

if delta > 1:
  print(f"*** RACE DETECTED: leaked {delta - 1} reference(s) ***")
  raise SystemExit(1)
if errors:
  print("errors during run:", errors[:3])
  raise SystemExit(2)
print("no race observed on this run")
"""


def main():
  N_TRIALS = 100
  leaks = 0
  for i in range(N_TRIALS):
      interp = interpreters.create()
      try:
          interp.exec(CHILD)
      except interpreters.ExecutionFailed as e:
          # SystemExit(1) inside the subinterpreter == race detected.
          print(f"trial {i}: {e}")
          leaks += 1
      finally:
          interp.close()
  print(f"\n{leaks}/{N_TRIALS} trials leaked")


if __name__ == "__main__":
  main()

Sample output (truncated):

baseline=1152921504606846978 after=1152921504606846979 delta=1 ... no race observed on this run
baseline=1152921504606846978 after=1152921504606846980 delta=2 ... *** RACE DETECTED: leaked 1
reference(s) ***
...
baseline=1152921504606846978 after=1152921504606846982 delta=4 ... *** RACE DETECTED: leaked 3
reference(s) ***
...
52/100 trials leaked