Following the discussion of #269, the behavior of asyncio.Semaphore now obeys the semantic of the semaphore.

• There is no guarantee that task A will acquire the semaphore before task B if A called semaphore.acquire() before B.

Now the implementation of Semaphore is the same as Queue's.

I am still thinking over fut.cancel() in Semaphore.acquire. Following the discussion of #269, I understand that the Future might be not done under some scenarios. According to the documentation of asyncio: (https://docs.python.org/3/library/asyncio-task.html)

result = await future or result = yield from future – suspends the coroutine until the future is done ...

Will assert fut.done() be another choice which could let the user notice that there might be something wrong?

I need a little time to review this. I think you're on the right track though!

I found that my comment above is wrong.

the behavior of asyncio.Semaphore now obeys the semantic of the semaphore.

asyncio.Semaphore is not exactly the same as normal semaphore. The sequence of the tasks acquiring the asyncio.Semaphore is deterministic under some scenarios.
For example, suppose there are only two tasks A and B. A is waiting for the semaphore sem. If we call sem.acquire() following sem.release() in task B, B will acquire sem before A. I'm not sure whether or not "changing the container of pending Futures to a list of randomly inserted elements" is superfluous.

Anyway, I should not say "asyncio.Semaphore now obeys the semantic of the semaphore."

Thanks! I've added some nits, let me know when you've addressed them and I'll merge this baby.

Oh, one more thing. You need to have an account on bugs.python.org and you need to sign the online PSF contributor form: https://www.python.org/psf/contrib/contrib-form/ I will then check your status in the bug tracker so we know you've signed it. (This only needs to happen for your first contribution to Python. :-)

@gvanrossum
I changed the condition of the while loop in acquire and restored the blank lines. And I created an account on bugs.python.org and signed the form.

Thank you very much for the review!

Oh and I think that when a pending task being deleted (raise GeneratorExit in Semaphore.acquire, Queue.put or Queue.get) there might be some bugs though ... I should explain what I concern more.

The generatorExit will be raised when close of the genenator underlying the wrapping task is called. This may happen when the semaphore and the event loop (and all the other things reference to the task) are destroyed, and the pending task is going to be destroyed. Maybe we don't need to worry about whether or not the state of the destroyed semaphore is correct.

On the other hand, the document says

result = await future or result = yield from future – suspends the coroutine until the future is done ...

A pending task will be awoken by a callback function _wakeup, which will call Task._step of the task. And _wakeup will be scheduled if and only if the corresponding future is done.

I am not sure whether or not "the spirit of yield from future implies that when we pass this line, we know the future is done".

We certainly can trust that when we pass yield from future without raising an exception the future is done. What we can't trust is whether it will be done when we catch an exeption, especially a BaseException instance (which is what GeneratorExit is). The philosophical question is then whether we might still care about the semaphore or queue when the generator is closed. I think it's at least conceivable that some code could create the generator, let it the event loop run for a little bit, and then decide to close the generator. So preserving the invariant of the queue or semaphore in this case sounds good. And that's what we do. So I'm going to merge your change!

Applied cc20f39.

I did a manual merge, so you may have to mark the PR as applied manually. Thanks!!

I apologize for sticking to this issue for several days. I want to make sure whether or not "the exceptions other than CancelledException should be take into account in other places using await future (or yield from future)."

... some code could create the generator, let it the event loop run for a little bit, and then decide to close the generator ...

I think that a generator G (or a coroutine) calling await future must be wrapped in a Task T scheduled by the event loop. To close the generator, maybe the programmer should call T.cancel() instead of calling close of underlying G (if the programmer can reference G). I can't figure out under what scenario a user of asyncio is able to and would like to operate the wrapped G directly.

When I was testing Queue, I got GeneratorExit when the program was going to exit and there are still some pending task which I didn't cancel before the program finished. I didn't know how to call close of the underlying generator without using Task._coro. As a user of asyncio, I think I should not use _coro with a leading underscore.

This is why I think getting the GeneratorExit here indicates the program may have some unexpected problems.

mark the PR as applied manually

Do I just leave a comment on this PR, or are there other steps I miss?

Here's a way that GeneratorExit can be caused:

from asyncio import coroutine, sleep, ensure_future, get_event_loop

@coroutine
def foo():
    try:
        yield from sleep(1)
    except BaseException as err:
        print('hoho')

@coroutine
def bar():
    coro = foo()
    task = ensure_future(coro)
    yield from sleep(0.1)
    coro.close()

get_event_loop().run_until_complete(bar())

However I don't think that this is a very realistic scenario. Another scenario is when you receive a KeyboardInterrupt. Since that is a BaseException, it barrels right past except Exception: clauses elsewhere in asyncio.

Looking back at it I think that's probably a mistake -- when we use try/finally, the finally action definition happens even when a BaseException is raised, but more explicit cleanup in except Exception: blocks ignores it. It would be nice if all fundamental internal data structures remained in a consistent state even when BaseException (of whatever nature -- GeneratorExit, SystemExit or KeyboardInterrupt) left things in a consistent state, just in case some later recovery process fires up the event loop again.

PS. I think just closing the issue was enough to take the PR off the list.

The thought that "calling coro.close() instead of task.cancel() in bar is a wrong use of asyncio" is what made me stick to GeneratorExit.

I think I'm too naive to think up these things relating to BaseException. Thank you very much for clarifying all of these issues!!!

It is wrong to do that. But if we can write the code robustly, so that we don't care about wrong things people do, without making it unreadable or obfuscated or slower, we should write the code robustly. And I think that's what we've accomplished. Do you agree?

Yes, I do. I learned a lot from the discussion! Thank you for all your help and guidance!