classification
Title: threading.Lock().release() raises _thread.error, not RuntimeError
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: aymeric.augustin, gregory.p.smith, gruszczy, pitrou
Priority: normal Keywords: patch

Created on 2011-02-07 15:35 by aymeric.augustin, last changed 2011-03-01 23:07 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
11140.patch gruszczy, 2011-02-27 17:37
11140.2.patch aymeric.augustin, 2011-03-01 07:26
Messages (9)
msg128128 - (view) Author: Aymeric Augustin (aymeric.augustin) * Date: 2011-02-07 15:35
The docs for the threading module state that:

> The release() method should only be called in the locked state; it changes the state to unlocked and returns immediately. If an attempt is made to release an unlocked lock, a RuntimeError will be raised.

However, I noticed that catching RuntimeError does not work. Actually release() raises thread.error, which inherits directly Exception.

I reproduced the behavior shown below in Python 2.6.6, 2.7.1 from MacPorts on Mac OS 10.6 and in Python 2.6.6 on Linux. The same happens in Python 3.2rc2 on Mac OS 10.6, except the thread module has been renamed to _thread so the exception becomes a _thread.error.

>>> import threading

>>> try:
...     threading.Lock().release()
... except RuntimeError:
...     pass
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
thread.error: release unlocked lock

>>> try:
...     threading.Lock().release()
... except Exception, e:
...     print type(e)
...     print type(e).mro()
... 
<class 'thread.error'>
[<class 'thread.error'>, <type 'exceptions.Exception'>, <type 'exceptions.BaseException'>, <type 'object'>]

I do not know if this must be fixed in the docs or the code. Currently, the type of the exception is probably platform-dependant since the thread module is not provided on all platforms.
msg128130 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-02-07 15:41
I would rather make _thread.error an alias of RuntimeError. That way, we can reconcile the docs and the code without breaking compatibility.
Also, importing _thread won't be necessary to catch errors raised in threading objects.
msg128131 - (view) Author: Aymeric Augustin (aymeric.augustin) * Date: 2011-02-07 15:55
I agree with your solution. Unfortunately, I do not know how you would redefine ThreadError to extend RuntimeError in a C extension.

_thread.error is created line 741 in trunk/Modules/threadmodule.c:
ThreadError = PyErr_NewException("thread.error", NULL, NULL);


dummy_thread.error is created lin 21 in trunk/Lib/dummy_thread.py:
class error(Exception):
    ...

Changing this to RuntimeException is trivial.


I don't think there are other implementations of threading in the Python source but I'd appreciate if someone more familiar with the interpreter could confirm.
msg128133 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-02-07 16:11
> aaugustin <aymeric.augustin@polyconseil.fr> added the comment:
> 
> I agree with your solution. Unfortunately, I do not know how you would
> redefine ThreadError to extend RuntimeError in a C extension.
> 
> _thread.error is created line 741 in trunk/Modules/threadmodule.c:
> ThreadError = PyErr_NewException("thread.error", NULL, NULL);

I was not proposing to extend it, just to alias it:

ThreadError = PyExc_RuntimeError;
Py_INCREF(ThreadError);

That said, extending is quite trivial if the behaviour is not changed:

ThreadError = PyErr_NewException("thread.error", PyExc_RuntimeError,
NULL);

(see
http://docs.python.org/dev/c-api/exceptions.html#PyErr_NewException)
msg129639 - (view) Author: Filip GruszczyƄski (gruszczy) Date: 2011-02-27 17:37
Test for releasing unacquired lock and aliasing exception.
msg129726 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-02-28 22:07
Patch committed in r88682, thank you!
msg129745 - (view) Author: Aymeric Augustin (aymeric.augustin) * Date: 2011-03-01 07:26
Shouldn't this be fixed in _dummy_thread too?

Attached patch contains fix + test.

(Unfortunately, I was not able to run the tests with my patch: `python3.2 -m test.regrtest test_dummy_thread` tests my 'system' version, not my svn checkout.  I did not change the ticket flags either because I do not know the rules here - first time I work on Python...)
msg129747 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-01 08:17
Hello,

> Shouldn't this be fixed in _dummy_thread too?
> 
> Attached patch contains fix + test.

Oops, thanks for noticing.

> (Unfortunately, I was not able to run the tests with my patch:
> `python3.2 -m test.regrtest test_dummy_thread` tests my 'system'
> version, not my svn checkout.

You must run "./python" instead and it should work.
msg129831 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-01 23:07
_dummy_thread patch committed in r88699, thank you!
History
Date User Action Args
2011-03-01 23:07:09pitrousetnosy: gregory.p.smith, pitrou, gruszczy, aymeric.augustin
messages: + msg129831
2011-03-01 08:17:32pitrousetnosy: gregory.p.smith, pitrou, gruszczy, aymeric.augustin
messages: + msg129747
2011-03-01 07:26:45aymeric.augustinsetfiles: + 11140.2.patch
nosy: gregory.p.smith, pitrou, gruszczy, aymeric.augustin
messages: + msg129745
2011-02-28 22:07:25pitrousetstatus: open -> closed
nosy: gregory.p.smith, pitrou, gruszczy, aymeric.augustin
messages: + msg129726

resolution: fixed
stage: needs patch -> resolved
2011-02-27 17:37:55gruszczysetfiles: + 11140.patch

nosy: + gruszczy
messages: + msg129639

keywords: + patch
2011-02-07 16:11:20pitrousetmessages: + msg128133
2011-02-07 15:55:47aymeric.augustinsetmessages: + msg128131
2011-02-07 15:41:14pitrousetversions: + Python 3.3, - Python 2.6, Python 2.7
type: behavior

nosy: + gregory.p.smith, pitrou
title: Error in the documentation of threading.Lock().release() -> threading.Lock().release() raises _thread.error, not RuntimeError
messages: + msg128130
stage: needs patch
2011-02-07 15:35:27aymeric.augustincreate