Skip to content

Commit f16a447

Browse files
committed
gh-109593: Fix reentrancy issue in multiprocessing resource_tracker
1 parent 8a82bff commit f16a447

File tree

6 files changed

+67
-1
lines changed

6 files changed

+67
-1
lines changed

Lib/multiprocessing/resource_tracker.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,14 @@
5454
class ResourceTracker(object):
5555

5656
def __init__(self):
57-
self._lock = threading.Lock()
57+
self._lock = threading.RLock()
5858
self._fd = None
5959
self._pid = None
6060

6161
def _stop(self):
6262
with self._lock:
63+
if self._lock._recursion_count() > 1:
64+
return
6365
if self._fd is None:
6466
# not running
6567
return
@@ -81,6 +83,8 @@ def ensure_running(self):
8183
This can be run from any process. Usually a child process will use
8284
the resource created by its parent.'''
8385
with self._lock:
86+
if self._lock._recursion_count() > 1:
87+
return
8488
if self._fd is not None:
8589
# resource tracker was launched before, is it still running?
8690
if self._check_alive():

Lib/test/lock_tests.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,42 @@ def test_release_save_unacquired(self):
330330
lock.release()
331331
self.assertRaises(RuntimeError, lock._release_save)
332332

333+
def test_recursion_count(self):
334+
lock = self.locktype()
335+
self.assertEqual(0, lock._recursion_count())
336+
lock.acquire()
337+
self.assertEqual(1, lock._recursion_count())
338+
lock.acquire()
339+
lock.acquire()
340+
self.assertEqual(3, lock._recursion_count())
341+
lock.release()
342+
self.assertEqual(2, lock._recursion_count())
343+
lock.release()
344+
lock.release()
345+
self.assertEqual(0, lock._recursion_count())
346+
347+
phase = []
348+
349+
def f():
350+
lock.acquire()
351+
phase.append(None)
352+
while len(phase) == 1:
353+
_wait()
354+
lock.release()
355+
phase.append(None)
356+
357+
with threading_helper.wait_threads_exit():
358+
start_new_thread(f, ())
359+
while len(phase) == 0:
360+
_wait()
361+
self.assertEqual(len(phase), 1)
362+
self.assertEqual(0, lock._recursion_count())
363+
phase.append(None)
364+
while len(phase) == 2:
365+
_wait()
366+
self.assertEqual(len(phase), 3)
367+
self.assertEqual(0, lock._recursion_count())
368+
333369
def test_different_thread(self):
334370
# Cannot release from a different thread
335371
lock = self.locktype()

Lib/test/test_importlib/test_locks.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ class ModuleLockAsRLockTests:
2929
test_timeout = None
3030
# _release_save() unsupported
3131
test_release_save_unacquired = None
32+
# _recursion_count() unsupported
33+
test_recursion_count = None
3234
# lock status in repr unsupported
3335
test_repr = None
3436
test_locked_repr = None

Lib/test/test_threading.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1779,6 +1779,9 @@ class ConditionAsRLockTests(lock_tests.RLockTests):
17791779
# Condition uses an RLock by default and exports its API.
17801780
locktype = staticmethod(threading.Condition)
17811781

1782+
def test_recursion_count(self):
1783+
self.skipTest("Condition does not expose _recursion_count()")
1784+
17821785
class ConditionTests(lock_tests.ConditionTests):
17831786
condtype = staticmethod(threading.Condition)
17841787

Lib/threading.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,13 @@ def _release_save(self):
245245
def _is_owned(self):
246246
return self._owner == get_ident()
247247

248+
# Internal method used for reentrancy checks
249+
250+
def _recursion_count(self):
251+
if self._owner != get_ident():
252+
return 0
253+
return self._count
254+
248255
_PyRLock = _RLock
249256

250257

Modules/_threadmodule.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,18 @@ PyDoc_STRVAR(rlock_release_save_doc,
490490
\n\
491491
For internal use by `threading.Condition`.");
492492

493+
static PyObject *
494+
rlock_recursion_count(rlockobject *self, PyObject *Py_UNUSED(ignored))
495+
{
496+
unsigned long tid = PyThread_get_thread_ident();
497+
return PyLong_FromUnsignedLong(
498+
self->rlock_owner == tid ? self->rlock_count : 0UL);
499+
}
500+
501+
PyDoc_STRVAR(rlock_recursion_count_doc,
502+
"_recursion_count() -> int\n\
503+
\n\
504+
For internal use by reentrancy checks.");
493505

494506
static PyObject *
495507
rlock_is_owned(rlockobject *self, PyObject *Py_UNUSED(ignored))
@@ -565,6 +577,8 @@ static PyMethodDef rlock_methods[] = {
565577
METH_VARARGS, rlock_acquire_restore_doc},
566578
{"_release_save", (PyCFunction)rlock_release_save,
567579
METH_NOARGS, rlock_release_save_doc},
580+
{"_recursion_count", (PyCFunction)rlock_recursion_count,
581+
METH_NOARGS, rlock_recursion_count_doc},
568582
{"__enter__", _PyCFunction_CAST(rlock_acquire),
569583
METH_VARARGS | METH_KEYWORDS, rlock_acquire_doc},
570584
{"__exit__", (PyCFunction)rlock_release,

0 commit comments

Comments
 (0)