New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document better what happens on releasing an unacquired lock #58707
Comments
From docs@python.org: """ Needing to know what would happen, I cautiously tested it out. I half I generally expect the documentation to tell me what will happen if I do I agree: if we know that a ThreadError will always be raised in this instance, we should document it as such. |
On Thu, Apr 5, 2012 at 09:06, Georg Brandl <report@bugs.python.org> wrote:
I've already prepared a small patch for that (every supported release |
What different exceptions are they? Note that thread.error == _thread.error == threading.ThreadError. The docs should always use the last one (ThreadError). |
Ah, and I missed that apparently on 3.3, _thread.Error is aliased to RuntimeError. In that case you should use RuntimeError of course :) |
New changeset efeca6ff2751 by Sandro Tosi in branch '2.7': New changeset acea9d95a6d8 by Sandro Tosi in branch '3.2': New changeset c10a0f93544e by Sandro Tosi in branch 'default': |
At least put the information inside some disclaimers about "normally"; even the stdlib has some fake locks that let you release a lock someone else holds. (I think I found them in in workarounds for threading not being available, such as the dummy_* modules, but still, it is possible.) |
Not sure what you're talking about. The doc patch is about unacquired Indeed the standard Lock object (but not the RLock) does allow releasing |
On Thu, Apr 5, 2012 at 5:38 PM, Antoine Pitrou <report@bugs.python.org> wrote:
Isn't one common reason for not being able to acquire a lock that |
New changeset 068a614e9d97 by Sandro Tosi in branch 'default': |
We're talking about *releasing* an (un)acquired lock, not acquiring it |
On Fri, Apr 6, 2012 at 5:57 AM, Antoine Pitrou <report@bugs.python.org> wrote:
Right, but I thought the original motivation was concern over a race lock.acquire()
try: # What if something happens here, during
try setup? Leak?
foo()
finally:
lock.release() vs
-jJ |
It doesn't matter *how* you get to the situation where you are releasing a lock that hasn't been acquired, the point is to document what actually happens when you do the release. And just yesterday I needed to know this, since I have a lock that may or may not be currently held when I release it, and now I know I can just catch RuntimeError in that case. |
Only if you're willing to make assumptions about the threading model and the source of locks. And I fear the current change overpromises. For example, the LockType from _dummy_thread raises an error not based on RuntimeError, and has comments suggesting it might stop raising entirely. I believe I have seen other Lock-emulation code which also does not raise an error, though the closest I can come to finding it right now is logging_releaseLock() when the import of either _thread or threading failed. Starting with http://hg.python.org/cpython/file/acea9d95a6d8/Doc/library/threading.rst I would prefer to change to following two sentences:
... in any of the following ways: (a) Change "will be"/"is" --> "may be", so it isn't promised:
... (b) Clarify that it is implementation-specific
... (and add to the caveats) (c) Clarify that alternatives are buggy (and fix those in the stdlib) (and add to the caveats) |
I, on the other hand, would prefer if it were made part of the API contract that an error is raised, and to fix any stdlib implementations *of that API* that don't conform to that. (That is, locks from other modules may well not follow that API, and their documentation should cover their API.) |
On Fri, Apr 6, 2012 at 10:32 PM, R. David Murray <report@bugs.python.org> wrote:
Do you consider it reasonable that all stdlib Locks follow that API, I don't feel comfortable declaring that (not even only for future -jJ |
I think dummy_threading should be fixed (but only in 3.3, just in case it causes any backward compatibility issues with someone's code). Logging I'd leave to Vinay to decide about. I'm assuming that if any of the others devs nosy on this issue disagree with me that they will speak up :) |
Re. logging, logging._acquireLock and logging._releaseLock are not part of the public API and are undocumented at present. The case when _releaseLock does not raise an error is when threading couldn't be imported, so the _lock variable is None. I don't see the need for adding any documentation for this. Logging doesn't use dummy_thread: if threading isn't available, all lock acquisition and release operations become no-ops. |
Vinay, The current question is what contract locks should follow, and whether -jJ |
I don't see the point of this discussion. We are talking about |
Agreed. Jim, I think you're trying to get consistency where none is required. |
The docs of 2.7 and 3.2 still first say that RuntimeError is raised, and then that a ThreadError is raised:
Lock.release()
...
When invoked on an unlocked lock, a ThreadError is raised. In 2.7 and 3.2, ThreadError is not a RuntimeError, so this is wrong. |
This is now fixed for 2.7 (see bpo-15829); no fix needed for 3.3+. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: