Problem with lock acquire non-blocking optimization about asyncio

[Martiusweb]

Hi,

Thanks for the detailed explanation! I understand the issue, but can't see why it needs to be fixed: if you use a lock in a coroutine, it's probably because you need to synchronize IO operations. They will be invoked with await/yield from and most likely yield to the scheduler.
In other words: in which "real-world" case do you think that an asyncio.Lock will be used without yielding?

Rather than yield from asyncio.sleep(0), you can use loop.call_soon(lock.release) to ensure that the lock will be released during the next loop iteration, which effectively makes yield from lock.acquire() to yield to the scheduler.

[u201701]

Thanks for taking the time to fully understand the issue. Typically application software fixes give significant weight to real-life use cases, whereas systems software such as libraries consider even remote cases. That's because there is a HUGE diversity of python users and if there is an issue, somebody or the other will encounter it. In other words, libraries such has this have to be a bit aspirational and strive to perfection.

That said, I dug deeper into this issue and found 2 things that make it moot.

1. Numerous other asyncio coroutines do not block in the fast path, including those used for mutual exclusions. Two random examples: Event.wait() and Queue.get().
   Thus, firstly, fixing just Lock is of little consequence -- users must still be cautious about many other coroutines.
   Secondly, I am not sure whether it is wise for all co-routines or even just mutual exclusion ones to be "fixed" so that they always yield to the scheduler, even in the fast path. In fact, I lean towards preserving the current fast paths.
   Thirdly, I am inclined to believe that there should always be a way to tell whether the coroutine will yield to scheduler or not.

2. There is indeed a method Lock.locked() that can predict whether Lock.acquire() will yield to scheduler or not, and Lock.locked() itself does not yield to the scheduler.
   This method can be used by minority users who may care about this condition. Hopefully other similar coroutines with a fast path also have similar predictor methods.

3. This distantly relates to an earlier debate in #284 (not involving me) regarding the behavior of asyncio.sleep(0). The call to sleep(t) was actually de-optimized (in the context of this issue) to yield to scheduler even though it could just return without yielding when t == 0. This was done to avoid adding a new method to the library.
   Given the above discussion, it seems like a bad idea to have done so. I hope in light of the discussion in this issue, somebody decides to go with the original thought to add async.nop rather than overload sleep in an unusual way.

4. Finally, I just noticed that the atypical behavior of asyncio.sleep is not even documented in the docstring. Hopefully the library maintainers at least fix this oversight.

Accordingly, I withdraw this issue. I do want to bring @1st1 's attention to this, especially regarding asyncio.nop(). Please close this issue at your convenience.

[ajdavis]

Closing this. It was an interesting discussion! I'll add, as I close it, that I made similar decisions implementing async.Queue. At first I wanted any statement in a coroutine like yield from q.get() or yield from q.put(data) to definitely yield to the scheduler and unblock any other waiting coroutine. Guido and Nikolay convinced me that it was better for q.get() and q.put() to take a fast path if possible and not unblock all waiting coroutines. In real life, coroutines do I/O and yield to the scheduler while they wait for the I/O to complete, and that naturally causes the sort of interleaved execution you expected in your code example.

[u201701]

Thanks for adding your explanation @ajdavis

I agree that all is fine given the presence of fast path functions such as Lock.locked() that can predict whether another coroutine will yield or take the fast path.

The anomaly of asyncio.sleep(0) does remain and I hope some day a asyncio.nop() is added or at least the behavior of sleep(0) is documented.

Thanks!