Skip to content
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2a4d4f3
Add some implementation documentation
exarkun Jun 27, 2022
c9e33c7
Move _blocking_on management into a context manager
exarkun Jun 27, 2022
c5912bf
Support re-entrant imports in _BlockingOnManager
exarkun Jun 30, 2022
27ae2f0
Apply formatting and comment changes from review
exarkun Nov 28, 2022
1fcc78b
Merge remote-tracking branch 'origin/main' into 91351-importlib-reent…
exarkun Nov 28, 2022
5fd15d8
Rename _BlockingOnManager.tid as suggested by review
exarkun Nov 28, 2022
7a24f2c
flip the first two arguments to _has_deadlock as suggested by review
exarkun Nov 28, 2022
08892b4
Mark up parameters following PEP 257 as suggested by review
exarkun Nov 28, 2022
59b53c0
rename the `_blocking_on` parameter as suggested by review
exarkun Nov 28, 2022
20007c5
further document motivation for `blocking_on` parameter as suggested …
exarkun Nov 28, 2022
bad1d3c
Rename more _has_deadlocked parameters as suggested by review
exarkun Nov 28, 2022
2821fcf
Treat None and [] the same for this case as suggested by review
exarkun Nov 28, 2022
95c73cb
update old comment to refer to new names as suggested by review
exarkun Nov 28, 2022
49ff9dd
Make _ModuleLock.count a list of True as suggested by review
exarkun Nov 28, 2022
cd174a8
Adjust the check for a module lock being released as suggest by review
exarkun Nov 28, 2022
719b181
Finish the _BlockingOnManager.tid renaming
exarkun Nov 28, 2022
6e809cd
Fix renaming of `_blocking_on` parameter to `_has_deadlocked`
exarkun Nov 28, 2022
74cbccd
Apply review suggestion
exarkun Jan 6, 2023
c579533
Apply review suggestion
exarkun Jan 6, 2023
decb70b
Apply review suggestion
exarkun Jan 6, 2023
3c91cc3
Apply review suggestion
exarkun Jan 6, 2023
e032ae2
Apply review suggestion
exarkun Jan 6, 2023
dba393a
Apply review suggestion
exarkun Jan 6, 2023
25d554b
Apply review suggestion
exarkun Jan 6, 2023
44157a9
Apply review suggestion
exarkun Jan 6, 2023
f99ed46
Apply review suggestion
exarkun Jan 6, 2023
b6d21f8
Apply review suggestion
exarkun Jan 6, 2023
5643442
Apply review suggestion
exarkun Jan 6, 2023
92036a8
Apply review suggestion
exarkun Jan 6, 2023
bf14ce2
Apply review suggestion
exarkun Jan 6, 2023
1cc6033
Apply review suggestion
exarkun Jan 6, 2023
ab40737
Apply review suggestion
exarkun Jan 6, 2023
36082eb
news blurb
exarkun Jan 6, 2023
55fc599
Merge branch 'main' into 91351-importlib-reentrancy
exarkun Jan 6, 2023
b35f0a8
Apply suggestions from code review
exarkun Jan 14, 2023
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
Next Next commit
Apply formatting and comment changes from review
Co-authored-by: Brett Cannon <brett@python.org>
  • Loading branch information
exarkun and brettcannon authored Nov 28, 2022
commit 27ae2f07ceb5e16191bdc8cb46b821edef7b742e
54 changes: 19 additions & 35 deletions Lib/importlib/_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,42 +66,25 @@ def _new_module(name):


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.
"""
"""A context manager responsible to updating ``_blocking_on``."""
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.
"""
"""Mark the running thread as waiting for self.lock. via _blocking_on."""
# 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
# re-entrant (i.e., 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, [])

# 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)

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


Expand All @@ -111,11 +94,12 @@ class _DeadlockError(RuntimeError):


def _has_deadlock(seen, subject, tids, _blocking_on):
"""Check if 'subject' is holding the same lock as another thread(s).

The search within _blocking_on starts with the threads listed in tids.
'seen' contains any threads that are considered already traversed in the search.

"""
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.
Expand All @@ -136,7 +120,7 @@ def _has_deadlock(seen, subject, tids, _blocking_on):

if tid in seen:
# bpo 38091: the chain of tid's we encounter here
# eventually leads to a fixpoint or a cycle, but
# eventually leads to a fixed point 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.
Expand All @@ -159,7 +143,7 @@ class _ModuleLock:

def __init__(self, name):
# Create an RLock for protecting the import process for the
# corresponding module. Since it is an RLock a single thread will be
# corresponding module. Since it is an RLock, a single thread will be
# able to take it more than once. This is necessary to support
# re-entrancy in the import system that arises from (at least) signal
# handlers and the garbage collector. Consider the case of:
Expand Down Expand Up @@ -188,15 +172,15 @@ def __init__(self, name):
# identifier for the owning thread.
self.owner = None

# This is a count of the number of times the owning thread has
# acquired this lock. This supports RLock-like ("re-entrant lock")
# Represent the number of times the owning thread has acquired this lock
# via a list of `True`. 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.
# and list.pop() are both atomic and thread-safe in CPython 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
Expand Down Expand Up @@ -261,7 +245,7 @@ def acquire(self):
# 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.
# 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
Expand All @@ -271,7 +255,7 @@ def acquire(self):
# 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)
raise _DeadlockError(f'deadlock detected by {self!r}')

# Check to see if we're going to be able to acquire the
# lock. If we are going to have to wait then increment
Expand All @@ -291,9 +275,9 @@ def acquire(self):
# thread holding this lock (self.owner) calls self.release.
self.wakeup.acquire()

# Taking it has served its purpose (making us wait) so we can
# Taking the lock 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.
# next iteration around this 'while' loop.
self.wakeup.release()

def release(self):
Expand Down