Open OnceUponATimeInAmerica opened 1 year ago
Can you explain better what you think it needs to do and what would need to change in the implementation? Is it just a matter of it it using asyncio.Lock
instead threading.RLock
at:
I wasn't even aware that asyncio had locks so know nothing about them and how they are used.
Thanks for the reply. I have to warn I am just a user.
To begin, asycio locks are awaited (their acquire
method is a coroutine and invoked byasync with ...
).
Also (I assume) they would not be reentrant as it would basically allow the single thread servicing the event loop (and executing most/all the coroutines in the program, perhaps even concurrently) to go basically anywhere without any restrictions.
And because wrapper of a coroutine itself needs to be a coroutine (a chain of await ...
) the syntax has to accommodate this. If we want to use the same decorator for both synchronous functions and also coroutines, the decorator function (which itself, of course, is an ordinary synchronous function) has to detect the type (of the wrapped routine) and return the appropriate wrapper. Somewhat similar to this or this.
Also if we couple this to the present feature in wrapt
that the same synchronized
decorator can be applied to both instance methods and also to static methods/class methods or even standalone functions (basically, it automatically decides the granularity for synchronization) the situation can get somewhat complex. Although the former problem (routine's type: sync vs. async) is orthogonal to the latter problem (instance vs class method).
Hope this was somewhat helpful.
Quick note on wrapping either sync-or-async, since I've done this in compose
: the only universal way to do this, last I checked, is to wait until you had the return value - because an async callable might be implemented as a normal sync function returning an "awaitable" value.
I think adapting wrapt.synchronized
to handle to asyncio coroutines is doable. As @mentalisttraceur mentions, the wrapper will need to perform runtime introspection on the wrapped function to determine if it is a synchronous or coroutine function. Also, if the semaphore is passed, it too would need to be identified as having synchronous or coroutine methods. @GrahamDumpleton If you see wrapt.synchronized
as having a solid future, I'd be willing to take a crack at adding asyncio support to it, if you want.
Even if you can come up with a separate asynchronized
decorator just to see if it is possible for async alone that would certainly be educational and help work out what to do and whether one could come up with a single decorator that does both. I have not played enough with Python async libraries to be sure that anything I worked out would be correct. So any help/guidance would be much appreciated.
OK, will work on it. Just paste here for now, or propose in a PR?
Just follow up here initially with any conclusions you come to. My feeling right now is that an asynchronized
decorator is only going to cover a subset of what synchronized
can be used for. The asynchronized
decorator might even need to use the synchronized
decorator, or more basic standard mutex locks, internally to gate updates to stuff.
Correction:
As @mentalisttraceur mentions, the wrapper will need to perform runtime introspection on the
wrapped functionto determine if it isa synchronous or coroutine function.
... on the return value of the wrapped function, to determine if it is an awaitable or not.
My point was that inspecting the function (as with inspect.iscoroutinefunction
or asyncio.iscoroutinefunction
) is an incomplete solution. It will miss cases such as
__call__
is async
(to also detect those ahead-of-time, you gotta do inspect.iscoroutinefunction(f) or inspect.iscoroutinefunction(f.__call__)
);async
but which still needs an await
because it returned an "awaitable" value (this is impossible to detect ahead-of-time, but can be detected at call time by calling the function synchronously and then doing an inspect.isawaitable
on the result).That last method will work 100% of the time in all cases, but it can be trickier (or in some cases impossible) to write the logic in a way that can only decide how to act after calling the possibly-async function.
Take a look at how I've got it implemented in acompose
, and then once that makes sense, also scroll down to the sacompose
variant (whose __call__
only sometimes returns an awaitable).
The downside of just using the isawaitable
method for all cases is that we end up with a wrapper that will always be false for the iscoroutinefunction
check - it loses the ahead-of-time check in cases where it could be possible.
So the ideal wrapper combines these technique. Here's an untested/incomplete sketch of what that might look like:
def wrapper(function):
if inspect.iscoroutinefunction(function) or inspect.iscoroutinefunction(function.__call__):
async def _wrapper(*args, **kwargs):
async with lock:
return function(*args, **kwargs)
else:
def _wrapper(*args, **kwargs):
result = function(*args, **kwargs)
if inspect.isawaitable(result):
async def _finish():
async with lock:
return await result
return _finish() # `await` intentionally omitted
else:
return result
return _wrapper
FWIW, old example which tries to deal with sync or async functions.
https://github.com/GrahamDumpleton/wrapt/issues/150#issuecomment-893232442
Based on comments above though, that too probably needs more work to be complete, but important in that example is that it still uses @wrapt.decorator
so introspection works on the resulting wrapped function.
!!! 3.12 added inspect.markcoroutinefunction
, arguably wiping out all my objections.
I think this tips the balance strongly in favor of: if a function returns an awaitable object but doesn't pass inspect.iscoroutinefunction
, that's a bug in that callable, not something you need to handle.
If I had to re-design acompose
and sacompose
today, in a world where every Python implementation has a way to make any callable pass inspect.iscoroutinefunction
, I would probably never bother with all that at-call-time inspect.isawaitable
stuff - if inspect.iscoroutinefunction(function) or inspect.iscoroutinefunction(function.__call__):
would be good enough.
Personally, I think it's fine for wrapt
to have async
detection which is basically perfect on Python 3.12+ and only fails for an obscure niche edge case on older Pythons.
(In fact, it might be possible to backport inspect.markcoroutinefunction
using ctypes
or a C extension to older Python versions, at least for CPython.)
Thanks for
wrapt
.I was wondering if you are going to add support for synchronizing coroutines (using
@synchronized
decorator) and acceptingasyncio Locks
to wrapt? Currently it does not accept asyncio locks. Also because ayncio model is mostly single-threaded but highly re-entrant, rlocks (which are more suitable for parallel scenarios) would not work (I think).