This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: SemLock acquire() keyword arg 'blocking' is invalid
Type: enhancement Stage: resolved
Components: Documentation Versions: Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: berker.peksag Nosy List: berker.peksag, davin, docs@python, python-dev, r.david.murray, rhettinger, sbt, td
Priority: normal Keywords: patch

Created on 2015-02-19 11:31 by td, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue_23484_doc_locks_py35_and_py34.patch davin, 2015-03-26 15:32 Ignore - replaced by updated patch file review
issue_23484_doc_locks_py27.patch davin, 2015-03-26 15:33 Ignore - replaced by updated patch file review
issue_23484_doc_locks_py35_and_py34_noverchange.patch davin, 2015-04-08 20:27 Change docs in default/3.5 and 3.4 (both branches) - updated review
issue_23484_doc_locks_py27_noverchange.patch davin, 2015-04-08 20:28 Change docs in 2.7 - updated review
Messages (17)
msg236215 - (view) Author: Teodor Dima (td) Date: 2015-02-19 11:31
The keyword for 'blocking' in the documentation for multiprocessing.Lock.acquire() (and all synchronization objects dependent on SemLock) differs from its implementation at Modules/_multiprocessing/semaphore.c:70 - https://docs.python.org/3.4/library/threading.html#threading.Lock.acquire
msg236332 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-02-20 21:27
Interesting!  The documentation in 3.4 as well as 2.7 indicates that the keyword should be 'blocking' yet the code implements this as 'block'.

Code to reproduce empirically what is actually implemented:
import multiprocessing
dummy_lock = multiprocessing.Lock()
dummy_lock.acquire(blocking=False)   # Raises a TypeError on invalid keyword

The same code changed to 'block=False' works happily.


The code should be changed to reflect the docs and a test probably added too that both exercises this keyword explicitly by name and tests to see if we've fallen out of sync with the threading module.
msg236348 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-02-21 03:26
Of course, there's code in the wild that expects and uses the parameter named 'block' so simply changing this keyword will result in breaking others' code.

Two potentially appealing options:
1) Document that acquire in multiprocessing differs from threading in this way.
2) Implement 'blocking' as a supported keyword argument though preserve support for 'block' as a deprecated keyword.
msg236431 - (view) Author: Teodor Dima (td) Date: 2015-02-23 08:47
>>> Of course, there's code in the wild that expects and uses the parameter named 'block' so simply changing this keyword will result in breaking others' code.

That is, indeed, the case with my company's code as well.
msg236986 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-03-01 21:53
In the docs for 2.7, multiprocessing.Lock refers to threading.Lock in which the signature for acquire looks like:

    threading.Lock.acquire([blocking])


However, in the current code in 2.7, Modules/_multiprocessing/semaphore.c implements multiprocessing.Lock.acquire to support two arguments:  'block' and 'timeout'.


In the docs for 3.4, threading.Lock.acquire now has both 'blocking' and 'timeout' arguments.  Notably, it specifies: "It is forbidden to specify a timeout when blocking is false."  This is indeed enforced by threading.Lock but not at all by multiprocessing.Lock.  In a 3.4.3 shell:

    >>> import multiprocessing
    >>> import threading
    >>> ml = multiprocessing.Lock()
    >>> ml.acquire(False, 20.0)
    True
    >>> ml.release()
    >>> tl = threading.Lock()
    >>> tl.acquire(False, 20.0)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: can't specify a timeout for a non-blocking call


In 2.7, acquire similarly permits a supplied value for timeout even when set to be non-blocking.

A further difference:  the code in Modules/_multiprocessing/semaphore.c appears to translate a timeout=None into an infinite wait whereas the threading implementation rejects any attempt to supply a None for timeout, demanding a float instead.  (In 3.4, threading uses timeout=-1 to request an infinite wait.)


Given these discrepancies between threading's and multiprocessing's implementations for Lock and given the difficulties in renaming an argument that can be supplied as a non-keyword parameter, the right thing to do at this point is to properly document multiprocessing.Lock's acquire as using 'block' and 'timeout'.
msg236991 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-03-01 22:20
To be fair, the docs in 2.7 do actually mention the use of 'block' instead of 'blocking' in acquire though it does so inside a "Note" block a bit later in the docs after first claiming that multiprocessing.Lock is a "clone".

In 3.4, that important detail has been inexplicably stripped from this distended "Note" block.


Ultimately, this use of the "Note" block seems to misplace important information that should appear in the description of the multiprocessing.Lock class and its siblings in the synchronization section.
msg239339 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-03-26 15:32
Attaching patches for 3.5/default, 3.4, and 2.7 which update the documentation on multiprocessing.Lock, RLock, Semaphore, and BoundedSemaphore to describe their actual implemented behavior, replacing the existing, misleading claim that they are clones of threading.Lock, etc., respectively.

Specific changes:
* Lock and RLock now have their acquire and release methods drawn out so as to best explain the significant behavioral differences (versus threading) in return values, potential exceptions being raised, and passing arguments.
* Semaphore and BoundedSemaphore are not as drawn out because their behaviors are indeed the same in every regard except for the difference in block-vs-blocking (and in the case of 2.7, timeout is novel in multiprocessing and doesn't appear until 3.2 in threading).
* In 3.4 and 3.5, the inaccurate note block at the end of the synchronization section no longer misrepresents the treatment of the timeout argument in threading analogs.

It is worth noting that in 3.x, the behavior around the timeout argument in multiprocessing.{Lock,RLock,Semaphore,BoundedSemaphore} already matches that of threading.Semaphore and BoundedSemaphore.  The threading module appears inconsistent in its use of the timeout argument when comparing Semaphore/BoundedSemaphore with Lock/RLock.  It would be interesting to investigate whether in 3.x this inconsistency within threading could be brought in line with the rest of multiprocessing.

In issue21342 the notion of unifying the implementations of these synchronization primitives across threading and multiprocessing is suggested.  If that is pursued, it would reasonably be attempted on default/3.5 and not backported to 2.7 (nor probably 3.4).  Thus this patch to update the documentation is still highly valuable for 2.7 and 3.4 and 3.5 (at least until such time as a unification effort might be undertaken in the case of 3.5).
msg239770 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-04-01 04:34
@berker: I would have said this should not be marked an enhancement as the proposed solution (in the patch) is to correct the errors in the documentation to accurately describe the current implemented behavior.  Does that make sense?

Whatever label we give it, do you think my logic (described in my comments on this issue) for why it is best to fix it this way makes sense to you?
msg239809 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-04-01 13:46
behavior vs enhancment for doc changes is really pretty meaningless/arbitrary, IMO.
msg239821 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-04-01 15:31
I use "enhancement" for non-trivial documentation patches.

Also, quoting from https://docs.python.org/devguide/triaging.html#type

"Also used for improvements in the documentation and test suite and for other refactorings."

In this case, your patch fixes a documentation issue and improving current documentation by documenting {Lock,RLock}.acquire() and release() methods properly (documentation improvements can go into bugfix branches).
msg239929 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-04-02 15:49
@berker:  Sadly, I've read those descriptions in triaging.html more than once and that part apparently did not stick in my head.  Hopefully it will now -- thanks.

@r.david:  Ok, cool -- I had been mentally associating more significance to one versus the other but that helps to understand.

As Berker correctly suspected, I would not have expected something labelled "Enhancement" to be allowed into the 2.7 branch so the explanation definitely helps.
msg240283 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-04-08 20:27
Updating patch for default/3.5 and 3.4 branches to remove versionchanged message on {Lock,RLock}.acquire per feedback from @berker.
msg240284 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-04-08 20:28
Same for 2.7 branch.
msg250449 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-09-11 04:53
@berker.peksag, @r.david.murray: Could I gently nudge one of you into committing the final versions of these patches?
msg251195 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-09-21 03:52
New changeset f3faf0f355e0 by Berker Peksag in branch '3.4':
Issue #23484: Document differences between synchronization primitives of
https://hg.python.org/cpython/rev/f3faf0f355e0

New changeset 6cd030099966 by Berker Peksag in branch '3.5':
Issue #23484: Document differences between synchronization primitives of
https://hg.python.org/cpython/rev/6cd030099966

New changeset 020377a15708 by Berker Peksag in branch 'default':
Issue #23484: Document differences between synchronization primitives of
https://hg.python.org/cpython/rev/020377a15708
msg251197 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-09-21 04:15
New changeset 92350a2a8b08 by Berker Peksag in branch '2.7':
Issue #23484: Document differences between synchronization primitives of
https://hg.python.org/cpython/rev/92350a2a8b08
msg251198 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-09-21 04:17
Thanks, Teodor and Davin!
History
Date User Action Args
2022-04-11 14:58:12adminsetgithub: 67672
2021-06-21 12:03:02iritkatriellinkissue21342 superseder
2015-09-21 04:17:00berker.peksagsetstatus: open -> closed
resolution: fixed
messages: + msg251198

stage: patch review -> resolved
2015-09-21 04:15:49python-devsetmessages: + msg251197
2015-09-21 03:52:10python-devsetnosy: + python-dev
messages: + msg251195
2015-09-11 09:52:30berker.peksagsetassignee: docs@python -> berker.peksag
versions: + Python 3.6
2015-09-11 04:53:03davinsetmessages: + msg250449
2015-04-08 20:28:24davinsetfiles: + issue_23484_doc_locks_py27_noverchange.patch

messages: + msg240284
2015-04-08 20:27:53davinsetfiles: + issue_23484_doc_locks_py35_and_py34_noverchange.patch

messages: + msg240283
2015-04-02 15:49:49davinsetmessages: + msg239929
2015-04-01 15:31:05berker.peksagsetmessages: + msg239821
2015-04-01 13:46:40r.david.murraysetnosy: + r.david.murray
messages: + msg239809
2015-04-01 04:34:09davinsetmessages: + msg239770
2015-03-31 02:40:03berker.peksagsetnosy: + berker.peksag
type: behavior -> enhancement
2015-03-26 15:37:48davinsetnosy: + rhettinger
2015-03-26 15:33:31davinsetfiles: + issue_23484_doc_locks_py27.patch
2015-03-26 15:32:40davinsetfiles: + issue_23484_doc_locks_py35_and_py34.patch
keywords: + patch
messages: + msg239339

stage: needs patch -> patch review
2015-03-02 21:03:46davinlinkissue21342 dependencies
2015-03-01 22:20:20davinsetmessages: + msg236991
2015-03-01 21:53:05davinsetnosy: + docs@python
messages: + msg236986

assignee: docs@python
components: + Documentation, - Extension Modules
2015-02-23 08:47:41tdsetmessages: + msg236431
2015-02-21 03:26:13davinsetmessages: + msg236348
2015-02-20 21:27:45davinsetstage: needs patch
messages: + msg236332
versions: + Python 2.7, Python 3.5
2015-02-20 09:39:24ned.deilysetnosy: + sbt, davin
2015-02-19 11:31:14tdcreate