The definition of threading.Semaphore is confusing (for all versions of python docs)

https://docs.python.org/2/library/threading.html#semaphore-objects

 acquire([blocking])

It currently states the following:

> When invoked without arguments: if the internal counter is larger than zero on entry, decrement it by one and return immediately. If it is zero on entry, block, waiting until some other thread has called release() to make it larger than zero. This is done with proper interlocking so that if multiple acquire() calls are blocked, release() will wake exactly one of them up. The implementation may pick one at random, so the order in which blocked threads are awakened should not be relied on. There is no return value in this case.

However, after testing it myself I found that is missing a crutial detail. Let's step through the docs:

> If the internal counter is larger than zero on entry, decrement it by one and return immediately.

This is exactly accurate and should stay the same

> If it is zero on entry, block, waiting until some other thread has called release() to make it larger than zero. This is done with proper interlocking so that if multiple acquire() calls are blocked, release() will wake exactly one of them up. The implementation may pick one at random, so the order in which blocked threads are awakened should not be relied on. There is no return value in this case.

This is extremely confusing and I would like to rewrite it as follows:

> If it is zero on entry block until awoken by a call to ``release()``. Once awoken, decrement the counter by 1. Exactly one thread will be awoken by a call to ``release()``. The order in which threads are awoken should not be relied on. ``None`` is returned in this case.

The major thing that was missing was that the **counter is decremented after the thread is awoken**. For instance, this code *generally* passes assertions:

```
#!/usr/bin/python2
import time
from threading import Thread, Semaphore

s = Semaphore(1)

def doit():
    s.acquire()
    print("did it")

th1 = Thread(target=doit)
th1.start()

th2 = Thread(target=doit)
th2.start()

time.sleep(0.2)

assert not th1.is_alive()
assert th2.is_alive()

s.release()
assert s._value == 1, "The value is increased to 1 MOMENTARILY"
start = time.time()
while sum([th2.is_alive(), th3.is_alive()]) > 1:
    assert time.time() - start < 0.5
    time.sleep(0.1)

assert s._value == 0, "when an aquire is awoken, THEN _value is decremented"
```

Obviously this behavior should not be relied on, but it gives a picture of what seems to be happening under the hood.

I came across this while trying to work through "The Little Book of Semaphores". After reading these docs I mistakingly thought that they didn't match Djestra's original semaphore since the values could not be negative. I now realize that while they may not match that implementation under the hood, they match it perfectly in practice since if you (for instance) ``acquire()`` 5 times and then ``release()`` 5 times the value of Semaphore._value will be the same when all is said and done.

Also, is "The order in which threads are awoken should not be relied on" actually true? Would it not be up to the OS specific scheduler? For instance, wouldn't the POSIX scheduler use a priority queue of some kind?

It may be desirable to change:

> Once awoken, decrement the counter by 1.

to

> Once awoke, decrement the counter (which is guaranteed to be positive) by 1.

Although this could be considered obvious since the counter can never go below zero.

The changes you are proposing sound reasonable to me.  Would you like to submit a pull request for them?

Gave it a shot!

New changeset a0374dd34aa25f0895195d388b5ceff43b121b00 by Andrew Svetlov (Garrett Berg) in branch 'master':
bpo-32208: update threading.Semaphore docs and add unit test (#4709)
https://github.com/python/cpython/commit/a0374dd34aa25f0895195d388b5ceff43b121b00

New changeset a04ca12e12b522850e7e9244c250754d3cd36f0a by Andrew Svetlov (Miss Islington (bot)) in branch '3.6':
bpo-32208: update threading.Semaphore docs and add unit test (GH-4709) (#4750)
https://github.com/python/cpython/commit/a04ca12e12b522850e7e9244c250754d3cd36f0a

According to the discussion at https://github.com/python/cpython/pull/4709#issuecomment-350055732 it was decided to not backport this to 2.7. Closing this as 'fixed'. Thank you, all!