Skip to content
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

bpo-34793: Drop old-style context managers in asyncio.locks #17533

Merged
merged 3 commits into from
Feb 1, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions Doc/whatsnew/3.9.rst
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,11 @@ Removed
of :pep:`442`. Patch by Joannah Nanjekye.
(Contributed by Joannah Nanjekye in :issue:`15088`)

* ``with (await asyncio.lock):`` and ``with (yield from asyncio.lock):`` statements are
not longer supported, use ``async with lock`` instead. The same is correct for
``asyncio.Condition`` and ``asyncio.Semaphore``.
(Contributed by Andrew Svetlov in :issue:`34793`.)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also replace the deprecation warning with changedversion in https://docs.python.org/3/library/asyncio-sync.html#boundedsemaphore

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done



Porting to Python 3.9
=====================
Expand Down
83 changes: 0 additions & 83 deletions Lib/asyncio/locks.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,96 +3,13 @@
__all__ = ('Lock', 'Event', 'Condition', 'Semaphore', 'BoundedSemaphore')

import collections
import types
import warnings

from . import events
from . import futures
from . import exceptions
from .import coroutines


class _ContextManager:
"""Context manager.

This enables the following idiom for acquiring and releasing a
lock around a block:

with (yield from lock):
<block>

while failing loudly when accidentally using:

with lock:
<block>

Deprecated, use 'async with' statement:
async with lock:
<block>
"""

def __init__(self, lock):
self._lock = lock

def __enter__(self):
# We have no use for the "as ..." clause in the with
# statement for locks.
return None

def __exit__(self, *args):
try:
self._lock.release()
finally:
self._lock = None # Crudely prevent reuse.


class _ContextManagerMixin:
def __enter__(self):
raise RuntimeError(
'"yield from" should be used as context manager expression')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep this? We can change the error message to say that async with should be used...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the motivation was emphasizing that context managers were supported but in a slightly unusual way: with (yield from lock):; bare with lock: was forbidden.

After switching to async with lock: everything becomes clear: with is not supported in any form for the sake of async with. The error message is standard and well-known: AttributeError: __enter__. I think we shouldn't replace the default behavior.

Maybe the standard text can be improved globally, but it is another story, I guess.


def __exit__(self, *args):
# This must exist because __enter__ exists, even though that
# always raises; that's how the with-statement works.
pass

@types.coroutine
def __iter__(self):
# This is not a coroutine. It is meant to enable the idiom:
#
# with (yield from lock):
# <block>
#
# as an alternative to:
#
# yield from lock.acquire()
# try:
# <block>
# finally:
# lock.release()
# Deprecated, use 'async with' statement:
# async with lock:
# <block>
warnings.warn("'with (yield from lock)' is deprecated "
"use 'async with lock' instead",
DeprecationWarning, stacklevel=2)
yield from self.acquire()
return _ContextManager(self)

# The flag is needed for legacy asyncio.iscoroutine()
__iter__._is_coroutine = coroutines._is_coroutine

async def __acquire_ctx(self):
await self.acquire()
return _ContextManager(self)

def __await__(self):
warnings.warn("'with await lock' is deprecated "
"use 'async with lock' instead",
DeprecationWarning, stacklevel=2)
# To make "with await lock" work.
return self.__acquire_ctx().__await__()

async def __aenter__(self):
await self.acquire()
# We have no use for the "as ..." clause in the with
Expand Down
165 changes: 34 additions & 131 deletions Lib/test/test_asyncio/test_locks.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,32 +47,24 @@ def test_repr(self):
self.assertTrue(repr(lock).endswith('[unlocked]>'))
self.assertTrue(RGX_REPR.match(repr(lock)))

with self.assertWarns(DeprecationWarning):
@asyncio.coroutine
def acquire_lock():
with self.assertWarns(DeprecationWarning):
yield from lock

self.loop.run_until_complete(acquire_lock())
self.loop.run_until_complete(lock.acquire())
self.assertTrue(repr(lock).endswith('[locked]>'))
self.assertTrue(RGX_REPR.match(repr(lock)))

def test_lock(self):
with self.assertWarns(DeprecationWarning):
lock = asyncio.Lock(loop=self.loop)


@asyncio.coroutine
def acquire_lock():
with self.assertWarns(DeprecationWarning):
return (yield from lock)

res = self.loop.run_until_complete(acquire_lock())
return (yield from lock)

self.assertTrue(res)
self.assertTrue(lock.locked())
with self.assertRaisesRegex(
TypeError,
"object is not iterable"
):
self.loop.run_until_complete(acquire_lock())

lock.release()
self.assertFalse(lock.locked())

def test_lock_by_with_statement(self):
Expand All @@ -90,13 +82,13 @@ def test_lock_by_with_statement(self):
def test(lock):
yield from asyncio.sleep(0.01)
self.assertFalse(lock.locked())
with self.assertWarns(DeprecationWarning):
with (yield from lock) as _lock:
self.assertIs(_lock, None)
self.assertTrue(lock.locked())
yield from asyncio.sleep(0.01)
self.assertTrue(lock.locked())
self.assertFalse(lock.locked())
with self.assertRaisesRegex(
TypeError,
"object is not iterable"
):
with (yield from lock):
pass
self.assertFalse(lock.locked())

for primitive in primitives:
loop.run_until_complete(test(primitive))
Expand Down Expand Up @@ -302,52 +294,16 @@ def test_release_no_waiters(self):
self.assertFalse(lock.locked())

def test_context_manager(self):
with self.assertWarns(DeprecationWarning):
lock = asyncio.Lock(loop=self.loop)
async def f():
lock = asyncio.Lock()
self.assertFalse(lock.locked())

@asyncio.coroutine
def acquire_lock():
with self.assertWarns(DeprecationWarning):
return (yield from lock)
async with lock:
self.assertTrue(lock.locked())

with self.loop.run_until_complete(acquire_lock()):
self.assertTrue(lock.locked())
self.assertFalse(lock.locked())

self.assertFalse(lock.locked())

def test_context_manager_cant_reuse(self):
with self.assertWarns(DeprecationWarning):
lock = asyncio.Lock(loop=self.loop)

@asyncio.coroutine
def acquire_lock():
with self.assertWarns(DeprecationWarning):
return (yield from lock)

# This spells "yield from lock" outside a generator.
cm = self.loop.run_until_complete(acquire_lock())
with cm:
self.assertTrue(lock.locked())

self.assertFalse(lock.locked())

with self.assertRaises(AttributeError):
with cm:
pass

def test_context_manager_no_yield(self):
with self.assertWarns(DeprecationWarning):
lock = asyncio.Lock(loop=self.loop)

try:
with lock:
self.fail('RuntimeError is not raised in with expression')
except RuntimeError as err:
self.assertEqual(
str(err),
'"yield from" should be used as context manager expression')

self.assertFalse(lock.locked())
self.loop.run_until_complete(f())


class EventTests(test_utils.TestCase):
Expand Down Expand Up @@ -809,33 +765,14 @@ def test_repr(self):
self.assertTrue(RGX_REPR.match(repr(cond)))

def test_context_manager(self):
with self.assertWarns(DeprecationWarning):
cond = asyncio.Condition(loop=self.loop)

with self.assertWarns(DeprecationWarning):
@asyncio.coroutine
def acquire_cond():
with self.assertWarns(DeprecationWarning):
return (yield from cond)

with self.loop.run_until_complete(acquire_cond()):
self.assertTrue(cond.locked())

self.assertFalse(cond.locked())

def test_context_manager_no_yield(self):
with self.assertWarns(DeprecationWarning):
cond = asyncio.Condition(loop=self.loop)

try:
with cond:
self.fail('RuntimeError is not raised in with expression')
except RuntimeError as err:
self.assertEqual(
str(err),
'"yield from" should be used as context manager expression')
async def f():
cond = asyncio.Condition()
self.assertFalse(cond.locked())
async with cond:
self.assertTrue(cond.locked())
self.assertFalse(cond.locked())

self.assertFalse(cond.locked())
self.loop.run_until_complete(f())

def test_explicit_lock(self):
with self.assertWarns(DeprecationWarning):
Expand Down Expand Up @@ -920,16 +857,14 @@ def test_semaphore(self):
with self.assertWarns(DeprecationWarning):
@asyncio.coroutine
def acquire_lock():
with self.assertWarns(DeprecationWarning):
return (yield from sem)
return (yield from sem)

res = self.loop.run_until_complete(acquire_lock())

self.assertTrue(res)
self.assertTrue(sem.locked())
self.assertEqual(0, sem._value)
with self.assertRaisesRegex(
TypeError,
"'Semaphore' object is not iterable",
):
self.loop.run_until_complete(acquire_lock())

sem.release()
self.assertFalse(sem.locked())
self.assertEqual(1, sem._value)

Expand Down Expand Up @@ -1064,38 +999,6 @@ def test_release_no_waiters(self):
sem.release()
self.assertFalse(sem.locked())

def test_context_manager(self):
with self.assertWarns(DeprecationWarning):
sem = asyncio.Semaphore(2, loop=self.loop)

@asyncio.coroutine
def acquire_lock():
with self.assertWarns(DeprecationWarning):
return (yield from sem)

with self.loop.run_until_complete(acquire_lock()):
self.assertFalse(sem.locked())
self.assertEqual(1, sem._value)

with self.loop.run_until_complete(acquire_lock()):
self.assertTrue(sem.locked())

self.assertEqual(2, sem._value)

def test_context_manager_no_yield(self):
with self.assertWarns(DeprecationWarning):
sem = asyncio.Semaphore(2, loop=self.loop)

try:
with sem:
self.fail('RuntimeError is not raised in with expression')
except RuntimeError as err:
self.assertEqual(
str(err),
'"yield from" should be used as context manager expression')

self.assertEqual(2, sem._value)


if __name__ == '__main__':
unittest.main()
13 changes: 6 additions & 7 deletions Lib/test/test_asyncio/test_pep492.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,12 @@ def test_context_manager_with_await(self):
async def test(lock):
await asyncio.sleep(0.01)
self.assertFalse(lock.locked())
with self.assertWarns(DeprecationWarning):
with await lock as _lock:
self.assertIs(_lock, None)
self.assertTrue(lock.locked())
await asyncio.sleep(0.01)
self.assertTrue(lock.locked())
self.assertFalse(lock.locked())
with self.assertRaisesRegex(
TypeError,
"can't be used in 'await' expression"
):
with await lock:
pass

for primitive in primitives:
self.loop.run_until_complete(test(primitive))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Remove support for ``with (await asyncio.lock):`` and ``with (yield from
asyncio.lock):``. The same is correct for ``asyncio.Condition`` and
``asyncio.Semaphore``.