Skip to content

gh-91351: Support re-entrancy in importlib/_bootstrap.py #94342

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.

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
292 changes: 256 additions & 36 deletions Lib/importlib/_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,169 @@ def _new_module(name):
# A dict mapping module names to weakrefs of _ModuleLock instances
# Dictionary protected by the global import lock
_module_locks = {}
# A dict mapping thread ids to _ModuleLock instances

# A dict mapping thread ids to lists of _ModuleLock instances. This maps a
# thread to the module locks it is blocking on acquiring. The values are
# lists because a single thread could perform a re-entrant import and be "in
# the process" of blocking on locks for more than one module. "in the
# process" because a thread cannot actually block on acquiring more than one
# lock but it can have set up bookkeeping that reflects that it intends to
# block on acquiring more than one lock.
_blocking_on = {}


class _BlockingOnManager:
"""
A context manager responsible to updating ``_blocking_on`` to track which
threads are likely blocked on taking the import locks for which modules.
"""
def __init__(self, tid, lock):
# The id of the thread in which this manager is being used.
self.tid = tid
# The _ModuleLock for a certain module which the running thread wants
# to take.
self.lock = lock

def __enter__(self):
"""
Mark the running thread as waiting for the lock this manager knows
about.

:return: A context manager which can be used to take and release the
lock for the _ModuleLock this manager knows about.
"""
# Interactions with _blocking_on are *not* protected by the global
# import lock here because each thread only touches the state that it
# owns (state keyed on its thread id). The global import lock is
# re-entrant (ie, a single thread may take it more than once) so it
# wouldn't help us be correct in the face of re-entrancy either.

# First look up the module locks the running thread already intends to
# take. If this thread hasn't done an import before, it may not be
# present in the dict so be sure to initialize it in this case.
self.blocked_on = _blocking_on.setdefault(self.tid, [])

# Start with a presumption that we're not re-entrant. In this case,
# we will return the regular module so lock _ModuleLock.acquire can
# take it. However, below, we might discover we are re-entrant and
# choose to do something else.
#
# A lot of logic here might be redundant if self.lock.lock were just
# an RLock.
reentrant = False

if self.blocked_on:
# In this case we are re-entering this acquire method. It is not
# only that we are doing a re-entrant import but we are
# re-entering *this method* to take a module import lock. This is
# possible if an import is triggered by the garbage collector, a
# signal handler, etc.

if self in self.blocked_on and self.lock.lock.locked():
# Not only are we re-entering this method to take the import
# lock but we're re-entering it for a _ModuleLock for which it
# is already running and for which the module lock is already
# held.
#
# Put another way, the call stack looks something like:
#
# import foo
# -> ...
# -> importlib._bootstrap._ModuleLock.acquire
# -> ...
# -> <garbage collector>
# -> __del__
# -> import foo
# -> ...
# -> importlib._bootstrap._ModuleLock.acquire
# -> _BlockingOnManager.__enter__
#
# We don't want to (and can't) take it again so put a dummy in
# its place.
reentrant = True

# Whether we are re-entering or not, add this lock to the list because
# now this thread is going to be blocked on it.
self.blocked_on.append(self.lock)

if reentrant:
# Since we can't take the original lock again, give back a context
# manager that does nothing instead. Access by the import system
# to the module is still protected by the fact that the real lock
# is already held and will be until the outer
# _ModuleLock.acquire/_ModuleLock.release process finishes. That
# necessarily happens after this re-entrant use finishes.
return _NoopManager()

# In the non-re-entrant case, give back the real module lock so it can
# be acquired and released to protect the module.
return self.lock.lock

def __exit__(self, *args, **kwargs):
"""
Mark the running thread as no longer waiting for the lock this manager
knows about.
"""
self.blocked_on.remove(self.lock)


class _NoopManager:
"""
A context manager that does nothing.
"""
def __enter__(self):
pass

def __exit__(self, *args, **kwargs):
pass


class _DeadlockError(RuntimeError):
pass



def _has_deadlock(seen, subject, tids, _blocking_on):
"""
Considering a graph where nodes are threads (represented by their id
as keys in ``blocking_on``) and edges are "blocked on" relationships
(represented by values in ``_blocking_on``), determine whether ``subject``
is reachable starting from any of the threads given by ``tids``.

:param seen: A set of threads that have already been visited.
:param subject: The thread id to try to reach.
:param tids: The thread ids from which to begin.
:param blocking_on: A dict representing the thread/blocking-on graph.
"""
if subject in tids:
# If we have already reached the subject, we're done - signal that it
# is reachable.
return True

# Otherwise, try to reach the subject from each of the given tids.
for tid in tids:
blocking_on = _blocking_on.get(tid)
if blocking_on is None:
# There are no edges out from this node, skip it.
continue

if tid in seen:
# bpo 38091: the chain of tid's we encounter here
# eventually leads to a fixpoint or a cycle, but
# does not reach 'me'. This means we would not
# actually deadlock. This can happen if other
# threads are at the beginning of acquire() below.
return False
seen.add(tid)

# Follow the edges out from this thread.
edges = [lock.owner for lock in blocking_on]
if _has_deadlock(seen, subject, edges, _blocking_on):
return True

return False


class _ModuleLock:
"""A recursive lock implementation which is able to detect deadlocks
(e.g. thread 1 trying to take locks A then B, and thread 2 trying to
Expand All @@ -58,31 +213,54 @@ class _ModuleLock:
def __init__(self, name):
self.lock = _thread.allocate_lock()
self.wakeup = _thread.allocate_lock()

# The name of the module for which this is a lock.
self.name = name

# Either None if this lock is not owned by any thread or the thread
# identifier for the owning thread.
self.owner = None
self.count = 0
self.waiters = 0

# This is a count of the number of times the owning thread has
# acquired this lock. This supports RLock-like ("re-entrant lock")
# behavior, necessary in case a single thread is following a circular
# import dependency and needs to take the lock for a single module
# more than once.
#
# Counts are represented as a list of None because list.append(None)
# and list.pop() are both atomic and thread-safe and it's hard to find
# another primitive with the same properties.
self.count = []

# This is a count of the number of threads that are blocking on
# `self.wakeup.acquire()` to try to get their turn holding this module
# lock. When the module lock is released, if this is greater than
# zero, it is decremented and `self.wakeup` is released one time. The
# intent is that this will let one other thread make more progress on
# acquiring this module lock. This repeats until all the threads have
# gotten a turn.
#
# This is incremented in `self.acquire` when a thread notices it is
# going to have to wait for another thread to finish.
#
# See the comment above count for explanation of the representation.
self.waiters = []

def has_deadlock(self):
# Deadlock avoidance for concurrent circular imports.
me = _thread.get_ident()
tid = self.owner
seen = set()
while True:
lock = _blocking_on.get(tid)
if lock is None:
return False
tid = lock.owner
if tid == me:
return True
if tid in seen:
# bpo 38091: the chain of tid's we encounter here
# eventually leads to a fixpoint or a cycle, but
# does not reach 'me'. This means we would not
# actually deadlock. This can happen if other
# threads are at the beginning of acquire() below.
return False
seen.add(tid)
# To avoid deadlocks for concurrent or re-entrant circular imports,
# look at the "blocking on" state to see if any threads are blocking
# on getting the import lock for any module for which the import lock
# is held by this thread.
return _has_deadlock(
seen=set(),
# Try to find this thread
subject=_thread.get_ident(),
# starting from the thread that holds the import lock for this
# module.
tids=[self.owner],
# using the global "blocking on" state.
_blocking_on=_blocking_on,
)

def acquire(self):
"""
Expand All @@ -91,35 +269,77 @@ def acquire(self):
Otherwise, the lock is always acquired and True is returned.
"""
tid = _thread.get_ident()
_blocking_on[tid] = self
try:
with _BlockingOnManager(tid, self) as the_lock:
while True:
with self.lock:
if self.count == 0 or self.owner == tid:
# Protect interaction with state on self with a per-module
# lock. This makes it safe for more than one thread to try to
# acquire the lock for a single module at the same time.
with the_lock:
if self.count == [] or self.owner == tid:
# If the lock for this module is unowned then we can
# take the lock immediately and succeed. If the lock
# for this module is owned by the running thread then
# we can also allow the acquire to succeed. This
# supports circular imports (thread T imports module A
# which imports module B which imports module A).
self.owner = tid
self.count += 1
self.count.append(None)
return True

# At this point we know the lock is held (because count !=
# 0) by another thread (because owner != tid). We'll have
# to get in line to take the module lock.

# But first, check to see if this thread would create a
# deadlock by acquiring this module lock. If it would
# then just stop with an error.
#
# XXX It's not clear who is expected to handle this error.
# There is one handler in _lock_unlock_module but many
# times this method is called when entering the context
# manager _ModuleLockManager instead - so _DeadlockError
# will just propagate up to application code.
#
# This seems to be more than just a hypothetical -
# https://stackoverflow.com/questions/59509154
# https://github.com/encode/django-rest-framework/issues/7078
if self.has_deadlock():
raise _DeadlockError('deadlock detected by %r' % self)

# Check to see if we're going to be able to acquire the
# lock. If we are going to have to wait then increment
# the waiters so `self.release` will know to unblock us
# later on. We do this part non-blockingly so we don't
# get stuck here before we increment waiters. We have
# this extra acquire call (in addition to the one below,
# outside the self.lock context manager) to make sure
# self.wakeup is held when the next acquire is called (so
# we block). This is probably needlessly complex and we
# should just take self.wakeup in the return codepath
# above.
if self.wakeup.acquire(False):
self.waiters += 1
# Wait for a release() call
self.waiters.append(None)

# Now blockingly take the lock. This won't complete until the
# thread holding this lock (self.owner) calls self.release.
self.wakeup.acquire()

# Taking it has served its purpose (making us wait) so we can
# give it up now. We'll take it non-blockingly again on the
# next iteration around this while loop.
self.wakeup.release()
finally:
del _blocking_on[tid]

def release(self):
tid = _thread.get_ident()
with self.lock:
if self.owner != tid:
raise RuntimeError('cannot release un-acquired lock')
assert self.count > 0
self.count -= 1
if self.count == 0:
assert len(self.count) > 0
self.count.pop()
if len(self.count) == 0:
self.owner = None
if self.waiters:
self.waiters -= 1
if len(self.waiters) > 0:
self.waiters.pop()
self.wakeup.release()

def __repr__(self):
Expand Down
Loading