Description

QuentinN42

opened on Jan 13, 2024

Issue body actions

Bug report

Bug description:

Issue

I'm using a simple script to test how contextlib.contextmanager and contextlib.asynccontextmanager works.

After some investigations, it seems that if we missmatch the sync or async context manager with the sync or async function, the context manager is not called well.

decorator \ func	sync	async
sync	Ok	closer executed before function
async	never awaited	Ok

import asyncio
import contextlib


@contextlib.contextmanager
def sync_cm():
    print("sync open")
    yield
    print("sync close")


@contextlib.asynccontextmanager
async def async_cm():
    print("async open")
    yield
    print("async close")


@sync_cm()
def sync_sync():
    print("sync_sync")


@sync_cm()
async def sync_async():
    print("sync_async")


@async_cm()
def async_sync():
    print("async_sync")


@async_cm()
async def async_async():
    print("async_async")


async def main():
    sync_sync()
    print("\n---\n")
    await async_async()
    print("\n---\n")
    async_sync()
    print("\n---\n")
    await sync_async()


asyncio.run(main())

The expected output is:

sync open
sync_sync
sync close

---

async open
async_async
async close

---

sync open
sync_async
sync close

---

async open
async_sync
async close

But for the two last cases, the output is:

/home/n42/tmp/2024-01-13-contextlib/main.py:44: RuntimeWarning: coroutine 'async_sync' was never awaited
  async_sync()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

---

sync open
sync close
sync_async

For the first one that means that the function decorated with @contextlib.contextmanager is not awaited, wich is normal.

Potential reason

But for the last one we have a more concerning output:

sync open
sync close
sync_async

This means that if you decorate an async function with @contextlib.contextmanager, the context manager is opened (sync open) then closed (sync close) and after that the async function is called (sync_async).

As it doesn't raise any error, it's hard to understand why it's not working...

Solution

Afer some investigations, I have found that we can use the asyncio.iscoroutinefunction function to check if the function is async or not and then use the right decorator.

Here is the code:

import asyncio
import contextlib
import functools


class AgnosticContextManager(contextlib._GeneratorContextManager):
    def __call__(self, func):
        if asyncio.iscoroutinefunction(func):

            @functools.wraps(func)
            async def async_wrapper(*args, **kwargs):
                with self._recreate_cm():
                    return await func(*args, **kwargs)

            return async_wrapper
        else:

            @functools.wraps(func)
            def wrapper(*args, **kwargs):
                with self._recreate_cm():
                    return func(*args, **kwargs)

            return wrapper


def agnosticcontextmanager(func):
    @functools.wraps(func)
    def helper(*args, **kwds):
        return AgnosticContextManager(func, args, kwds)

    return helper


@agnosticcontextmanager
def agno_cm():
    print("agno open")
    yield
    print("agno close")


@agno_cm()
def sync():
    print("sync")


@agno_cm()
async def async_():
    print("async")


async def main():
    sync()
    print("\n---\n")
    await async_()


asyncio.run(main())

That gives the following results :

agno open
sync
agno close

---

agno open
async
agno close

I'm wondering if it can be implemented in the standard library to, by default close the resource after the function is awaited, even if the function is async.

Note: this solution works for sync function decorators, for async functions decorators, we need to update the with into async with in the two wrappers.

CPython versions tested on:

3.8, 3.10, 3.11, 3.12, 3.13, Cpython main branch

Operating systems tested on:

Linux

Reacted by ento

Activity

1. QuentinN42

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Jan 13, 2024
2. AlexWaygood

   added

   stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory

   on Jan 13, 2024
3. sunmy2019 commented on Jan 13, 2024

   

   sunmy2019

   on Jan 13, 2024

   Member

   More actions

   it seems that if we missmatch the sync or async context manager with the sync or async function, the context manager is not called well.

   This is expected. That's the reason they both exist.

   by default close the resource after the function is awaited, even if the function is async.

   I think you have misunderstood this.

   Resources are always closed. It's the coroutine that is left out.

   I'm wondering if it can be implemented in the standard library to

   This is a feature request, not a bug. It's nice to have, but also too trivial to add. I have no option on this.
4. sunmy2019

   added

   pendingThe issue will be closed if no feedback is providedThe issue will be closed if no feedback is provided

   on Jan 13, 2024
5. QuentinN42 commented on Jan 14, 2024

   

   QuentinN42

   on Jan 14, 2024

   Author

   More actions

   What do you think of implementing on the contextlib.contextmanager decorator the ability to decorate both sync and async functions, then if the resultant decorator is used on sync or async function it always work as expected ?

   I've successfully implemented it on my personal fork, I can create a PR if you want.
6. sunmy2019 commented on Jan 14, 2024

   

   sunmy2019

   on Jan 14, 2024

   Member

   More actions

   I recommend you post it here and see people's feedback. All feature requests go here.

   https://discuss.python.org/c/ideas/6

   Reacted by QuentinN42
7. serhiy-storchaka commented on Jan 15, 2024

   

   serhiy-storchaka

   on Jan 15, 2024

   Member

   More actions

   There is a reason why asynccontextmanager was added. Your agnosticcontextmanager will not work if the decorated function has other decorator.

   @agno_cm()
   @otherdecorator
   async def async_():
       print("async")

   Even if async_() is an asynchronous function, otherdecorator(async_) may return a synchronous function that returns an awaitable. Your agnosticcontextmanager will fail to recognize it as asynchronouse. So the behavior will depend on the implementation detail of otherdecorator.

   Reacted by QuentinN42
8. aabmass commented on Jan 18, 2024

   

   aabmass

   on Jan 18, 2024

   More actions

   Even if async_() is an asynchronous function, otherdecorator(async_) may return a synchronous function that returns an awaitable. Your agnosticcontextmanager will fail to recognize it as asynchronouse. So the behavior will depend on the implementation detail of otherdecorator.

   I think inspect.markcoroutinefunction is intended for exactly this purpose. For some precedent, the stdlib unittest.mock.patch already does something similar to the feature request here:
   

   cpython/Lib/unittest/mock.py

   Lines 1369 to 1374 in 7fa511b

   	def __call__(self, func):
   	if isinstance(func, type):
   	return self.decorate_class(func)
   	if inspect.iscoroutinefunction(func):
   	return self.decorate_async_callable(func)
   	return self.decorate_callable(func)
9. picnixz

   added

   type-featureA feature request or enhancementA feature request or enhancement

   and removed

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   pendingThe issue will be closed if no feedback is providedThe issue will be closed if no feedback is provided

   on Dec 2, 2024

Issue actions