bpo-42392: Improve removal of *loop* parameter in asyncio primitives (GH-23499)

* Update code after merge review from 1st1

* Use a sentinel approach for loop parameter
Remove unnecessary _get_running_loop patching

* Use more clear function name (_verify_parameter_is_marker -> _verify_no_loop)

* Add init method to _LoopBoundMixin to check that loop param wasn't used
This commit is contained in:
Yurii Karabas 2020-11-25 13:50:44 +02:00 committed by GitHub
parent 7301979b23
commit b9127dd6ee
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 46 additions and 35 deletions

View File

@ -19,7 +19,7 @@ class _ContextManagerMixin:
self.release() self.release()
class Lock(_ContextManagerMixin, mixins._LoopBoundedMixin): class Lock(_ContextManagerMixin, mixins._LoopBoundMixin):
"""Primitive lock objects. """Primitive lock objects.
A primitive lock is a synchronization primitive that is not owned A primitive lock is a synchronization primitive that is not owned
@ -73,7 +73,8 @@ class Lock(_ContextManagerMixin, mixins._LoopBoundedMixin):
""" """
def __init__(self): def __init__(self, *, loop=mixins._marker):
super().__init__(loop=loop)
self._waiters = None self._waiters = None
self._locked = False self._locked = False
@ -153,7 +154,7 @@ class Lock(_ContextManagerMixin, mixins._LoopBoundedMixin):
fut.set_result(True) fut.set_result(True)
class Event(mixins._LoopBoundedMixin): class Event(mixins._LoopBoundMixin):
"""Asynchronous equivalent to threading.Event. """Asynchronous equivalent to threading.Event.
Class implementing event objects. An event manages a flag that can be set Class implementing event objects. An event manages a flag that can be set
@ -162,7 +163,8 @@ class Event(mixins._LoopBoundedMixin):
false. false.
""" """
def __init__(self): def __init__(self, *, loop=mixins._marker):
super().__init__(loop=loop)
self._waiters = collections.deque() self._waiters = collections.deque()
self._value = False self._value = False
@ -214,7 +216,7 @@ class Event(mixins._LoopBoundedMixin):
self._waiters.remove(fut) self._waiters.remove(fut)
class Condition(_ContextManagerMixin, mixins._LoopBoundedMixin): class Condition(_ContextManagerMixin, mixins._LoopBoundMixin):
"""Asynchronous equivalent to threading.Condition. """Asynchronous equivalent to threading.Condition.
This class implements condition variable objects. A condition variable This class implements condition variable objects. A condition variable
@ -224,7 +226,8 @@ class Condition(_ContextManagerMixin, mixins._LoopBoundedMixin):
A new Lock object is created and used as the underlying lock. A new Lock object is created and used as the underlying lock.
""" """
def __init__(self, lock=None): def __init__(self, lock=None, *, loop=mixins._marker):
super().__init__(loop=loop)
if lock is None: if lock is None:
lock = Lock() lock = Lock()
elif lock._loop is not self._get_loop(): elif lock._loop is not self._get_loop():
@ -328,7 +331,7 @@ class Condition(_ContextManagerMixin, mixins._LoopBoundedMixin):
self.notify(len(self._waiters)) self.notify(len(self._waiters))
class Semaphore(_ContextManagerMixin, mixins._LoopBoundedMixin): class Semaphore(_ContextManagerMixin, mixins._LoopBoundMixin):
"""A Semaphore implementation. """A Semaphore implementation.
A semaphore manages an internal counter which is decremented by each A semaphore manages an internal counter which is decremented by each
@ -343,7 +346,8 @@ class Semaphore(_ContextManagerMixin, mixins._LoopBoundedMixin):
ValueError is raised. ValueError is raised.
""" """
def __init__(self, value=1): def __init__(self, value=1, *, loop=mixins._marker):
super().__init__(loop=loop)
if value < 0: if value < 0:
raise ValueError("Semaphore initial value must be >= 0") raise ValueError("Semaphore initial value must be >= 0")
self._value = value self._value = value
@ -406,9 +410,9 @@ class BoundedSemaphore(Semaphore):
above the initial value. above the initial value.
""" """
def __init__(self, value=1): def __init__(self, value=1, *, loop=mixins._marker):
self._bound_value = value self._bound_value = value
super().__init__(value) super().__init__(value, loop=loop)
def release(self): def release(self):
if self._value >= self._bound_value: if self._value >= self._bound_value:

View File

@ -5,10 +5,20 @@ from . import events
_global_lock = threading.Lock() _global_lock = threading.Lock()
# Used as a sentinel for loop parameter
_marker = object()
class _LoopBoundedMixin:
class _LoopBoundMixin:
_loop = None _loop = None
def __init__(self, *, loop=_marker):
if loop is not _marker:
raise TypeError(
f'As of 3.10, the *loop* parameter was removed from '
f'{type(self).__name__}() since it is no longer necessary'
)
def _get_loop(self): def _get_loop(self):
loop = events._get_running_loop() loop = events._get_running_loop()
@ -17,5 +27,5 @@ class _LoopBoundedMixin:
if self._loop is None: if self._loop is None:
self._loop = loop self._loop = loop
if loop is not self._loop: if loop is not self._loop:
raise RuntimeError(f'{type(self).__name__} have already bounded to another loop') raise RuntimeError(f'{self!r} is bound to a different event loop')
return loop return loop

View File

@ -17,7 +17,7 @@ class QueueFull(Exception):
pass pass
class Queue(mixins._LoopBoundedMixin): class Queue(mixins._LoopBoundMixin):
"""A queue, useful for coordinating producer and consumer coroutines. """A queue, useful for coordinating producer and consumer coroutines.
If maxsize is less than or equal to zero, the queue size is infinite. If it If maxsize is less than or equal to zero, the queue size is infinite. If it
@ -29,7 +29,8 @@ class Queue(mixins._LoopBoundedMixin):
interrupted between calling qsize() and doing an operation on the Queue. interrupted between calling qsize() and doing an operation on the Queue.
""" """
def __init__(self, maxsize=0): def __init__(self, maxsize=0, *, loop=mixins._marker):
super().__init__(loop=loop)
self._maxsize = maxsize self._maxsize = maxsize
# Futures. # Futures.

View File

@ -51,6 +51,23 @@ class LockTests(test_utils.TestCase):
self.assertFalse(lock.locked()) self.assertFalse(lock.locked())
def test_lock_doesnt_accept_loop_parameter(self):
primitives_cls = [
asyncio.Lock,
asyncio.Condition,
asyncio.Event,
asyncio.Semaphore,
asyncio.BoundedSemaphore,
]
for cls in primitives_cls:
with self.assertRaisesRegex(
TypeError,
rf'As of 3.10, the \*loop\* parameter was removed from '
rf'{cls.__name__}\(\) since it is no longer necessary'
):
cls(loop=self.loop)
def test_lock_by_with_statement(self): def test_lock_by_with_statement(self):
loop = asyncio.new_event_loop() # don't use TestLoop quirks loop = asyncio.new_event_loop() # don't use TestLoop quirks
self.set_event_loop(loop) self.set_event_loop(loop)

View File

@ -541,31 +541,10 @@ class TestCase(unittest.TestCase):
self.set_event_loop(loop) self.set_event_loop(loop)
return loop return loop
def unpatch_get_running_loop(self):
events._get_running_loop = self._get_running_loop
def setUp(self): def setUp(self):
self._get_running_loop = events._get_running_loop
def _get_running_loop():
frame = sys._getframe(1)
if frame.f_globals['__name__'] == 'asyncio.mixins':
# When we called from LoopBoundedMixin we should
# fallback to default implementation of get_running_loop
try:
return events.get_running_loop()
except RuntimeError:
return None
return None
events._get_running_loop = _get_running_loop
self._thread_cleanup = threading_helper.threading_setup() self._thread_cleanup = threading_helper.threading_setup()
def tearDown(self): def tearDown(self):
self.unpatch_get_running_loop()
events.set_event_loop(None) events.set_event_loop(None)
# Detect CPython bug #23353: ensure that yield/yield-from is not used # Detect CPython bug #23353: ensure that yield/yield-from is not used