From 16d2a2196cc1e05f9c10bac8e668a5d445e4835b Mon Sep 17 00:00:00 2001 From: Samat K Jain <_@skj.io> Date: Tue, 25 Apr 2023 15:17:00 -0700 Subject: [PATCH 01/12] gh-70795: Rework RLock documentation Attempted to simultaneously reduce verbosity, while more descriptively describing behavior. Fix links (RLock acquire/release previously linking to Lock acquire/release, seems like bad copy pasta). --- Doc/library/threading.rst | 64 +++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 83ed48052704fb..d465ded350053a 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -594,14 +594,19 @@ and "recursion level" in addition to the locked/unlocked state used by primitive locks. In the locked state, some thread owns the lock; in the unlocked state, no thread owns it. -To lock the lock, a thread calls its :meth:`~RLock.acquire` method; this -returns once the thread owns the lock. To unlock the lock, a thread calls -its :meth:`~Lock.release` method. :meth:`~Lock.acquire`/:meth:`~Lock.release` -call pairs may be nested; only the final :meth:`~Lock.release` (the -:meth:`~Lock.release` of the outermost pair) resets the lock to unlocked and -allows another thread blocked in :meth:`~Lock.acquire` to proceed. +To lock the lock, a thread calls its :meth:`~RLock.acquire` method. To unlock +the lock, a thread calls its :meth:`~Lock.release` method. Reentrant locks +support the :ref:`context management protocol ` so `with` can be used. -Reentrant locks also support the :ref:`context management protocol `. +RLock's :meth:`~RLock.acquire`/:meth:`~RLock.release` call pairs may be nested, +unlike Lock's :meth:`~Lock.acquire`/:meth:`~Lock.release`. Only the final +:meth:`~RLock.release` (the :meth:`~Lock.release` of the outermost pair) resets +the lock to an unlocked state and allows another thread blocked in +:meth:`~RLock.acquire` to proceed. + +:meth:`~RLock.acquire`/:meth:`~RLock.release` must be in pairs: each acquire +must have a release in the thread that has acquired the lock. Failing to +release as many times the lock has required will lead to deadlock. .. class:: RLock() @@ -620,25 +625,38 @@ Reentrant locks also support the :ref:`context management protocol ` Acquire a lock, blocking or non-blocking. - When invoked without arguments: if this thread already owns the lock, increment - the recursion level by one, and return immediately. Otherwise, if another - thread owns the lock, block until the lock is unlocked. Once the lock is - unlocked (not owned by any thread), then grab ownership, set the recursion level - to one, and return. If more than one thread is blocked waiting until the lock - is unlocked, only one at a time will be able to grab ownership of the lock. - There is no return value in this case. + When invoked with the *blocking* argument set to ``True`` (the default): + + 1. If another thread owns the lock, block until we are able to acquire + lock, or *timeout*, if set. + + 2. If no thread owns the lock, acquire the lock and return immediately. + + 3. If the same thread owns the lock, acquire the lock again, and + return immediately. This is the difference between ``Lock`` and + ``RLock``. Internally, the "recursion level" is atomically + incremented to allow this. + + When invoked with the *blocking* argument set to ``False``, do not block. + Without blocking: + + 1. If another thread owns the lock, return immediately. - When invoked with the *blocking* argument set to ``True``, do the same thing as when - called without arguments, and return ``True``. + 2. If no thread owns the lock, acquire the lock and return immediately. - When invoked with the *blocking* argument 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``. + 3. If the same thread owns the lock. acquire the lock again and return + immediately. When invoked with the floating-point *timeout* argument set to a positive - value, block for at most the number of seconds specified by *timeout* - and as long as the lock cannot be acquired. Return ``True`` if the lock has - been acquired, ``False`` if the timeout has elapsed. + value, block for at most the number of seconds specified by *timeout*. + + In all cases, if the thread was able to acquire the lock, return ``True``. + If the thread was unable to acquire the lock (i.e. if *blocking* is False, or + is reached timeout) return ``False``. + + If called multiple times, failing to call :meth:`~RLock.release` as many times + may lead to deadlock. Consider using ``RLock`` as a context manager rather than + calling acquire/release directly. .. versionchanged:: 3.2 The *timeout* parameter is new. @@ -654,7 +672,7 @@ Reentrant locks also support the :ref:`context management protocol ` Only call this method when the calling thread owns the lock. A :exc:`RuntimeError` is raised if this method is called when the lock is - unlocked. + un-acquired. There is no return value. From cc594c1d4503ac13082db94f085a2c71ad50d24e Mon Sep 17 00:00:00 2001 From: Samat K Jain <_@skj.io> Date: Tue, 25 Apr 2023 15:20:49 -0700 Subject: [PATCH 02/12] Double backticks for with --- Doc/library/threading.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index d465ded350053a..331bfae7a153bc 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -596,7 +596,7 @@ no thread owns it. To lock the lock, a thread calls its :meth:`~RLock.acquire` method. To unlock the lock, a thread calls its :meth:`~Lock.release` method. Reentrant locks -support the :ref:`context management protocol ` so `with` can be used. +support the :ref:`context management protocol ` so ```with``` can be used. RLock's :meth:`~RLock.acquire`/:meth:`~RLock.release` call pairs may be nested, unlike Lock's :meth:`~Lock.acquire`/:meth:`~Lock.release`. Only the final From adc0823aa391a9f30c2403a4cd71785f9c15100d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?u=C4=B1=C9=90=C9=BE=20=CA=9E=20=CA=87=C9=90=C9=AF=C9=90s?= <_@skj.io> Date: Tue, 25 Apr 2023 16:39:22 -0700 Subject: [PATCH 03/12] Update Doc/library/threading.rst Co-authored-by: C.A.M. Gerlach --- Doc/library/threading.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 331bfae7a153bc..7cab8000901ce3 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -655,7 +655,7 @@ release as many times the lock has required will lead to deadlock. is reached timeout) return ``False``. If called multiple times, failing to call :meth:`~RLock.release` as many times - may lead to deadlock. Consider using ``RLock`` as a context manager rather than + may lead to deadlock. Consider using :class:`!RLock` as a context manager rather than calling acquire/release directly. .. versionchanged:: 3.2 From 2f0866a810868991088e3a9b0b6df2ec0e521abf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?u=C4=B1=C9=90=C9=BE=20=CA=9E=20=CA=87=C9=90=C9=AF=C9=90s?= <_@skj.io> Date: Tue, 25 Apr 2023 16:39:42 -0700 Subject: [PATCH 04/12] Update Doc/library/threading.rst Co-authored-by: C.A.M. Gerlach --- Doc/library/threading.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 7cab8000901ce3..e4c6264097a7fa 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -652,7 +652,7 @@ release as many times the lock has required will lead to deadlock. In all cases, if the thread was able to acquire the lock, return ``True``. If the thread was unable to acquire the lock (i.e. if *blocking* is False, or - is reached timeout) return ``False``. + *timeout* is reached or is non-positive) return ``False``. If called multiple times, failing to call :meth:`~RLock.release` as many times may lead to deadlock. Consider using :class:`!RLock` as a context manager rather than From d39b41cd7e164b7bf51a9ad2e9d8e9f70652b25f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?u=C4=B1=C9=90=C9=BE=20=CA=9E=20=CA=87=C9=90=C9=AF=C9=90s?= <_@skj.io> Date: Tue, 25 Apr 2023 17:02:52 -0700 Subject: [PATCH 05/12] Apply suggestions from code review Co-authored-by: C.A.M. Gerlach --- Doc/library/threading.rst | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index e4c6264097a7fa..bad8f9e4a2b899 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -594,9 +594,10 @@ and "recursion level" in addition to the locked/unlocked state used by primitive locks. In the locked state, some thread owns the lock; in the unlocked state, no thread owns it. -To lock the lock, a thread calls its :meth:`~RLock.acquire` method. To unlock -the lock, a thread calls its :meth:`~Lock.release` method. Reentrant locks -support the :ref:`context management protocol ` so ```with``` can be used. +Threads call a lock's :meth:`~RLock.acquire` method to lock it, +and its :meth:`~Lock.release` method to unlock it. Reentrant locks +support the :ref:`context management protocol ` so :keyword:`with` can be used +to acquire and release the lock automatically in a block of code. RLock's :meth:`~RLock.acquire`/:meth:`~RLock.release` call pairs may be nested, unlike Lock's :meth:`~Lock.acquire`/:meth:`~Lock.release`. Only the final @@ -604,9 +605,9 @@ unlike Lock's :meth:`~Lock.acquire`/:meth:`~Lock.release`. Only the final the lock to an unlocked state and allows another thread blocked in :meth:`~RLock.acquire` to proceed. -:meth:`~RLock.acquire`/:meth:`~RLock.release` must be in pairs: each acquire +:meth:`~RLock.acquire`/:meth:`~RLock.release` must be used in pairs: each acquire must have a release in the thread that has acquired the lock. Failing to -release as many times the lock has required will lead to deadlock. +call release as many times the lock has been acquired can lead to deadlock. .. class:: RLock() @@ -628,28 +629,24 @@ release as many times the lock has required will lead to deadlock. When invoked with the *blocking* argument set to ``True`` (the default): 1. If another thread owns the lock, block until we are able to acquire - lock, or *timeout*, if set. + lock, or *timeout*, if set to a positive float value. 2. If no thread owns the lock, acquire the lock and return immediately. 3. If the same thread owns the lock, acquire the lock again, and - return immediately. This is the difference between ``Lock`` and - ``RLock``. Internally, the "recursion level" is atomically - incremented to allow this. + return immediately. This is the difference between :class:`Lock` and + :class:`!RLock`; :class:`Lock` handles this case the same as the previous, + blocking until the lock can be acquired. - When invoked with the *blocking* argument set to ``False``, do not block. - Without blocking: + When invoked with the *blocking* argument set to ``False``: 1. If another thread owns the lock, return immediately. 2. If no thread owns the lock, acquire the lock and return immediately. - 3. If the same thread owns the lock. acquire the lock again and return + 3. If the same thread owns the lock, acquire the lock again and return immediately. - When invoked with the floating-point *timeout* argument set to a positive - value, block for at most the number of seconds specified by *timeout*. - In all cases, if the thread was able to acquire the lock, return ``True``. If the thread was unable to acquire the lock (i.e. if *blocking* is False, or *timeout* is reached or is non-positive) return ``False``. @@ -672,7 +669,7 @@ release as many times the lock has required will lead to deadlock. Only call this method when the calling thread owns the lock. A :exc:`RuntimeError` is raised if this method is called when the lock is - un-acquired. + not acquired. There is no return value. From 6a815ec64acf6552b9447e1ebbb3f490fefb6242 Mon Sep 17 00:00:00 2001 From: Samat K Jain <_@skj.io> Date: Tue, 25 Apr 2023 17:10:49 -0700 Subject: [PATCH 06/12] suggestion from gpshead --- Doc/library/threading.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index bad8f9e4a2b899..c83e762cf7b5f7 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -648,8 +648,8 @@ call release as many times the lock has been acquired can lead to deadlock. immediately. In all cases, if the thread was able to acquire the lock, return ``True``. - If the thread was unable to acquire the lock (i.e. if *blocking* is False, or - *timeout* is reached or is non-positive) return ``False``. + If the thread was unable to acquire the lock (i.e. if not blocking or + the timeout was reached) return ``False``. If called multiple times, failing to call :meth:`~RLock.release` as many times may lead to deadlock. Consider using :class:`!RLock` as a context manager rather than From f398ed13932159d2c579979d17df223791abd49e Mon Sep 17 00:00:00 2001 From: Samat K Jain <_@skj.io> Date: Tue, 25 Apr 2023 17:18:15 -0700 Subject: [PATCH 07/12] seealso for with-locks --- Doc/library/threading.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index c83e762cf7b5f7..bbfb46096ff7db 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -599,6 +599,10 @@ and its :meth:`~Lock.release` method to unlock it. Reentrant locks support the :ref:`context management protocol ` so :keyword:`with` can be used to acquire and release the lock automatically in a block of code. +.. seealso:: + + :ref:`with-locks`. + RLock's :meth:`~RLock.acquire`/:meth:`~RLock.release` call pairs may be nested, unlike Lock's :meth:`~Lock.acquire`/:meth:`~Lock.release`. Only the final :meth:`~RLock.release` (the :meth:`~Lock.release` of the outermost pair) resets From 82c7a651a45983b5f87b42ab5af8823f87a71e31 Mon Sep 17 00:00:00 2001 From: Samat K Jain <_@skj.io> Date: Tue, 25 Apr 2023 17:21:00 -0700 Subject: [PATCH 08/12] switch to bullet points --- Doc/library/threading.rst | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index bbfb46096ff7db..783cb81e62369d 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -632,24 +632,24 @@ call release as many times the lock has been acquired can lead to deadlock. When invoked with the *blocking* argument set to ``True`` (the default): - 1. If another thread owns the lock, block until we are able to acquire - lock, or *timeout*, if set to a positive float value. + * If another thread owns the lock, block until we are able to acquire + lock, or *timeout*, if set to a positive float value. - 2. If no thread owns the lock, acquire the lock and return immediately. + * If no thread owns the lock, acquire the lock and return immediately. - 3. If the same thread owns the lock, acquire the lock again, and - return immediately. This is the difference between :class:`Lock` and - :class:`!RLock`; :class:`Lock` handles this case the same as the previous, - blocking until the lock can be acquired. + * If the same thread owns the lock, acquire the lock again, and + return immediately. This is the difference between :class:`Lock` and + :class:`!RLock`; :class:`Lock` handles this case the same as the previous, + blocking until the lock can be acquired. When invoked with the *blocking* argument set to ``False``: - 1. If another thread owns the lock, return immediately. + * If another thread owns the lock, return immediately. - 2. If no thread owns the lock, acquire the lock and return immediately. + * If no thread owns the lock, acquire the lock and return immediately. - 3. If the same thread owns the lock, acquire the lock again and return - immediately. + * If the same thread owns the lock, acquire the lock again and return + immediately. In all cases, if the thread was able to acquire the lock, return ``True``. If the thread was unable to acquire the lock (i.e. if not blocking or From e35445ffaa96fae8d1b3ecf3d4340189ffe4df3e Mon Sep 17 00:00:00 2001 From: Samat K Jain <_@skj.io> Date: Tue, 25 Apr 2023 17:40:43 -0700 Subject: [PATCH 09/12] re-order bullet points --- Doc/library/threading.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 783cb81e62369d..cbe8e7f7229e8f 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -632,11 +632,11 @@ call release as many times the lock has been acquired can lead to deadlock. When invoked with the *blocking* argument set to ``True`` (the default): + * If no thread owns the lock, acquire the lock and return immediately. + * If another thread owns the lock, block until we are able to acquire lock, or *timeout*, if set to a positive float value. - * If no thread owns the lock, acquire the lock and return immediately. - * If the same thread owns the lock, acquire the lock again, and return immediately. This is the difference between :class:`Lock` and :class:`!RLock`; :class:`Lock` handles this case the same as the previous, @@ -644,10 +644,10 @@ call release as many times the lock has been acquired can lead to deadlock. When invoked with the *blocking* argument set to ``False``: - * If another thread owns the lock, return immediately. - * If no thread owns the lock, acquire the lock and return immediately. + * If another thread owns the lock, return immediately. + * If the same thread owns the lock, acquire the lock again and return immediately. From e9e744b619467cf728fa05b0386a8934dc28495e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?u=C4=B1=C9=90=C9=BE=20=CA=9E=20=CA=87=C9=90=C9=AF=C9=90s?= <_@skj.io> Date: Tue, 25 Apr 2023 18:04:11 -0700 Subject: [PATCH 10/12] Apply suggestions from code review Co-authored-by: C.A.M. Gerlach --- Doc/library/threading.rst | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index cbe8e7f7229e8f..3d52d221b54044 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -595,13 +595,19 @@ locks. In the locked state, some thread owns the lock; in the unlocked state, no thread owns it. Threads call a lock's :meth:`~RLock.acquire` method to lock it, -and its :meth:`~Lock.release` method to unlock it. Reentrant locks -support the :ref:`context management protocol ` so :keyword:`with` can be used -to acquire and release the lock automatically in a block of code. +and its :meth:`~Lock.release` method to unlock it. + +.. note:: + + Reentrant locks support the :ref:`context management protocol `, + so it is recommended to use :keyword:`with` instead of manually calling + :meth:`~RLock.acquire` and :meth:`~RLock.release` + to handle acquiring and releasing the lock for a block of code. .. seealso:: - :ref:`with-locks`. + :ref:`Using RLock as a context manager ` is recommended + over manual :meth:`!acquire` and :meth:`release` calls whenever practical. RLock's :meth:`~RLock.acquire`/:meth:`~RLock.release` call pairs may be nested, unlike Lock's :meth:`~Lock.acquire`/:meth:`~Lock.release`. Only the final From 77c155143989b3dc4cb9944a8da9561d8e093f09 Mon Sep 17 00:00:00 2001 From: Samat K Jain <_@skj.io> Date: Tue, 25 Apr 2023 18:09:12 -0700 Subject: [PATCH 11/12] move seealso section --- Doc/library/threading.rst | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 3d52d221b54044..b81968068e478c 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -604,11 +604,6 @@ and its :meth:`~Lock.release` method to unlock it. :meth:`~RLock.acquire` and :meth:`~RLock.release` to handle acquiring and releasing the lock for a block of code. -.. seealso:: - - :ref:`Using RLock as a context manager ` is recommended - over manual :meth:`!acquire` and :meth:`release` calls whenever practical. - RLock's :meth:`~RLock.acquire`/:meth:`~RLock.release` call pairs may be nested, unlike Lock's :meth:`~Lock.acquire`/:meth:`~Lock.release`. Only the final :meth:`~RLock.release` (the :meth:`~Lock.release` of the outermost pair) resets @@ -636,6 +631,12 @@ call release as many times the lock has been acquired can lead to deadlock. Acquire a lock, blocking or non-blocking. + .. seealso:: + + :ref:`Using RLock as a context manager ` is recommended + over manual :meth:`!acquire` and :meth:`release` calls whenever practical. + + When invoked with the *blocking* argument set to ``True`` (the default): * If no thread owns the lock, acquire the lock and return immediately. From 90b3559d90cbdcb7a605d7dc6bbeb7a0805a8de8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?u=C4=B1=C9=90=C9=BE=20=CA=9E=20=CA=87=C9=90=C9=AF=C9=90s?= <_@skj.io> Date: Tue, 25 Apr 2023 18:14:26 -0700 Subject: [PATCH 12/12] Update Doc/library/threading.rst Co-authored-by: C.A.M. Gerlach --- 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 b81968068e478c..4620a2120f4044 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -633,8 +633,9 @@ call release as many times the lock has been acquired can lead to deadlock. .. seealso:: - :ref:`Using RLock as a context manager ` is recommended - over manual :meth:`!acquire` and :meth:`release` calls whenever practical. + :ref:`Using RLock as a context manager ` + Recommended over manual :meth:`!acquire` and :meth:`release` calls + whenever practical. When invoked with the *blocking* argument set to ``True`` (the default):