From d52740ecd5ca82207f72345e068e84291b234714 Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Mon, 4 Dec 2017 17:38:40 -0700 Subject: [PATCH 1/4] fix issue32208: update threading.Semaphore docs and add unit test to validate correct behavior --- Doc/library/threading.rst | 25 +++++++++---------- Lib/test/test_threading.py | 50 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 95e27cab7c8e75d..e144155fdb8bdaa 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -684,8 +684,8 @@ Semaphores also support the :ref:`context management protocol `. .. class:: Semaphore(value=1) - This class implements semaphore objects. A semaphore manages a counter - representing the number of :meth:`release` calls minus the number of + This class implements semaphore objects. A semaphore manages an atomic + counter representing the number of :meth:`release` calls minus the number of :meth:`acquire` calls, plus an initial value. The :meth:`acquire` method blocks if necessary until it can return without making the counter negative. If not given, *value* defaults to 1. @@ -701,19 +701,18 @@ Semaphores also support the :ref:`context management protocol `. Acquire a semaphore. - 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 - :meth:`~Semaphore.release` to make it larger than zero. This is done - with proper interlocking so that if multiple :meth:`acquire` calls are - blocked, :meth:`~Semaphore.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. Returns - true (or blocks indefinitely). + When invoked without arguments: + - If the internal counter is larger than zero on entry, decrement it by + one and return true immediately. + - If the internal counter is zero on entry, block until awoken by a call to + :meth:`~Semaphore.release`. Once awoken (and the counter is greater + than 0), decrement the counter by 1 and return true. Exactly one + thread will be awoken by each call to ``release()``. The order in + which threads are awoken should not be relied on. When invoked with *blocking* set to false, do not block. If a call - without an argument would block, return false immediately; otherwise, - do the same thing as when called without arguments, and return true. + without an argument would block, return false immediately; otherwise, do + the same thing as when called without arguments, and return true. When invoked with a *timeout* other than ``None``, it will block for at most *timeout* seconds. If acquire does not complete successfully in diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 007581d7213b1f0..4e160fed073941c 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -656,6 +656,56 @@ def test_BoundedSemaphore_limit(self): t.join() self.assertRaises(ValueError, bs.release) + def test_semaphore_docs(self): + """Basic test to validate that the behavior described in the docs is + accurate. + """ + s = threading.Semaphore(1) + + # note: no mutex since we know only one thread is executing at once + results = [] + + def put_result(): + results.append(s.acquire()) + + th1 = threading.Thread(target=put_result) + th2 = threading.Thread(target=put_result) + th3 = threading.Thread(target=put_result) + + th1.start() + th2.start() + th3.start() + + start = time.time() + while th1.is_alive(): + assert time.time() - start < 0.5 + time.sleep(0.05) + + s.release() + while sum([th2.is_alive(), th3.is_alive()]) == 2: + assert time.time() - start < 0.5 + time.sleep(0.05) + + assert sum([th2.is_alive(), th3.is_alive()]) == 1 + assert s._value == 0 + + s.release() + while sum([th2.is_alive(), th3.is_alive()]) == 1: + assert time.time() - start < 0.5 + time.sleep(0.05) + + assert sum([th2.is_alive(), th3.is_alive()]) == 0 + assert s._value == 0 + + assert all([v == True for v in results]) + + s.release() + assert s._value == 1 + + # not bounded + s.release() + assert s._value == 2 + @cpython_only def test_frame_tstate_tracing(self): # Issue #14432: Crash when a generator is created in a C thread that is From 33fa11b58905e029e47962f343eef9bbbf953a9e Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Mon, 4 Dec 2017 21:12:34 -0700 Subject: [PATCH 2/4] add test for blocking --- Doc/library/threading.rst | 4 ++-- Lib/test/test_threading.py | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index e144155fdb8bdaa..517f6e2c1ab780e 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -707,8 +707,8 @@ Semaphores also support the :ref:`context management protocol `. - If the internal counter is zero on entry, block until awoken by a call to :meth:`~Semaphore.release`. Once awoken (and the counter is greater than 0), decrement the counter by 1 and return true. Exactly one - thread will be awoken by each call to ``release()``. The order in - which threads are awoken should not be relied on. + thread will be awoken by each call to :meth:`~Semaphore.release`. The + order in which threads are awoken should not be relied on. When invoked with *blocking* set to false, do not block. If a call without an argument would block, return false immediately; otherwise, do diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 4e160fed073941c..98cdab9e1e9131f 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -706,6 +706,10 @@ def put_result(): s.release() assert s._value == 2 + assert s.aquire(blocking=False) == True + assert s.aquire(blocking=False) == True + assert s.aquire(blocking=False) == False + @cpython_only def test_frame_tstate_tracing(self): # Issue #14432: Crash when a generator is created in a C thread that is From 519fd20b707fa9267397483468baa486f5e3ffa9 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 5 Dec 2017 10:24:27 +0200 Subject: [PATCH 3/4] Update threading.rst --- Doc/library/threading.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 517f6e2c1ab780e..b94021b4eb8f898 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -702,9 +702,10 @@ Semaphores also support the :ref:`context management protocol `. Acquire a semaphore. When invoked without arguments: - - If the internal counter is larger than zero on entry, decrement it by + + * If the internal counter is larger than zero on entry, decrement it by one and return true immediately. - - If the internal counter is zero on entry, block until awoken by a call to + * If the internal counter is zero on entry, block until awoken by a call to :meth:`~Semaphore.release`. Once awoken (and the counter is greater than 0), decrement the counter by 1 and return true. Exactly one thread will be awoken by each call to :meth:`~Semaphore.release`. The From 908a0a971a513d9f932dffbdec3c9d46aa1de0a7 Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Wed, 6 Dec 2017 09:46:40 -0700 Subject: [PATCH 4/4] semaphore: remove documentation validation tests and move 'return value' test to BaseSemaphore --- Lib/test/lock_tests.py | 6 +++-- Lib/test/test_threading.py | 54 -------------------------------------- 2 files changed, 4 insertions(+), 56 deletions(-) diff --git a/Lib/test/lock_tests.py b/Lib/test/lock_tests.py index a1ea96d42ce8106..5b1f033c6f806d5 100644 --- a/Lib/test/lock_tests.py +++ b/Lib/test/lock_tests.py @@ -629,13 +629,14 @@ def test_acquire_contended(self): sem = self.semtype(7) sem.acquire() N = 10 + sem_results = [] results1 = [] results2 = [] phase_num = 0 def f(): - sem.acquire() + sem_results.append(sem.acquire()) results1.append(phase_num) - sem.acquire() + sem_results.append(sem.acquire()) results2.append(phase_num) b = Bunch(f, 10) b.wait_for_started() @@ -659,6 +660,7 @@ def f(): # Final release, to let the last thread finish sem.release() b.wait_for_finished() + self.assertEqual(sem_results, [True] * (6 + 7 + 6 + 1)) def test_try_acquire(self): sem = self.semtype(2) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 98cdab9e1e9131f..007581d7213b1f0 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -656,60 +656,6 @@ def test_BoundedSemaphore_limit(self): t.join() self.assertRaises(ValueError, bs.release) - def test_semaphore_docs(self): - """Basic test to validate that the behavior described in the docs is - accurate. - """ - s = threading.Semaphore(1) - - # note: no mutex since we know only one thread is executing at once - results = [] - - def put_result(): - results.append(s.acquire()) - - th1 = threading.Thread(target=put_result) - th2 = threading.Thread(target=put_result) - th3 = threading.Thread(target=put_result) - - th1.start() - th2.start() - th3.start() - - start = time.time() - while th1.is_alive(): - assert time.time() - start < 0.5 - time.sleep(0.05) - - s.release() - while sum([th2.is_alive(), th3.is_alive()]) == 2: - assert time.time() - start < 0.5 - time.sleep(0.05) - - assert sum([th2.is_alive(), th3.is_alive()]) == 1 - assert s._value == 0 - - s.release() - while sum([th2.is_alive(), th3.is_alive()]) == 1: - assert time.time() - start < 0.5 - time.sleep(0.05) - - assert sum([th2.is_alive(), th3.is_alive()]) == 0 - assert s._value == 0 - - assert all([v == True for v in results]) - - s.release() - assert s._value == 1 - - # not bounded - s.release() - assert s._value == 2 - - assert s.aquire(blocking=False) == True - assert s.aquire(blocking=False) == True - assert s.aquire(blocking=False) == False - @cpython_only def test_frame_tstate_tracing(self): # Issue #14432: Crash when a generator is created in a C thread that is