msg91198 - (view) |
Author: Nikolaus Rath (nikratio) * |
Date: 2009-08-02 20:02 |
When threads are created by a C extension loaded with ctypes,
threading.local() objects are always empty. If one uses
_threading_local.local() instead of threading.local(), the problem does
not occur.
More information and example program showing the behaviour:
http://code.google.com/p/fusepy/issues/detail?id=15
|
msg99414 - (view) |
Author: Nikolaus Rath (nikratio) * |
Date: 2010-02-16 15:02 |
Here is a simple testcase:
$ gcc -fPIC -g -c -Wall lib.c
$ gcc -shared -Wl -o lib.so lib.o
$ python test.py
In callback: creating thread
Python callback_fn called, setting lc.x = 42
Python callback_fn called, setting lc.x = 42
Expected output:
$ python test.py
In callback: creating thread
Python callback_fn called, setting lc.x = 42
Python callback_fn called, lc.x = 42
|
msg99415 - (view) |
Author: Nikolaus Rath (nikratio) * |
Date: 2010-02-16 15:06 |
Note also that if the from __future__ import is enabled, the program segfaults instead (separate bug)?
|
msg116671 - (view) |
Author: Swapnil Talekar (swapnil) |
Date: 2010-09-17 14:43 |
As far as I know, the thread creation done in the file is not correct. While creating threads in C extension, there are certain rules to follow. Firstly, Python should be made thread-aware if it is not already i.e. call PyEval_InitThreads in the C callback function. After its creation, the thread should bootstrap to be able to execute Python code. It should create a new PyThreadState for itself by calling PyThreadState_New. For this, the thread should be passed the InterpreterState* through the entry function. Before executing any Python code, the thread should make sure that the current ThreadState * is corresponding to it by calling PyEval_RestoreThread.
Refer http://docs.python.org/c-api/init.html#thread-state-and-the-global-interpreter-lock
|
msg116672 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2010-09-17 15:08 |
Swapnil's analysis looks correct to me - there are certain rules you have to follow before calling back into the Python interpreter core. If you don't follow them, the behaviour you will get is completely undefined.
If the problem still occurs even when the C thread is correctly initialised for calling into the Python C API then this issue can be reopened.
|
msg116673 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2010-09-17 15:28 |
One more thing:
in the sample code, the thread initializes no PyThreadState. So the ctypes callback creates a temporary PyThreadState just for the duration of the call.
This explains the difference between threading.local and _threading_local:
- threading.local() uses the PyThreadState to store its data: You get a new value on each call.
- _threading_local.local() uses a global dictionary indexed by the thread id: the same value is retained across call.
|
msg116710 - (view) |
Author: Nikolaus Rath (nikratio) * |
Date: 2010-09-17 18:58 |
@Swapnil: the rules you quote are correct for the C extension, but do not apply when using ctypes, because ctypes is doing the required initializations automatically.
However, if Amaury is correct, ctypes performs the initializations in a way that break the threading.local functionality.
I think the best way to address this bug would therefore be to add a warning to the ctypes documentation that C created threads will not support threading.local().
|
msg116720 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2010-09-17 20:49 |
> ctypes performs the initializations in a way that break the
> threading.local functionality.
Not quite. ctypes (or more exactly: the PyGILState_Ensure() and PyGILState_Release() functions, which are the standard API to do this) is in a situation where it must call Python code from a thread which has no PyThreadState. So it creates a thread state, runs the code, and destroys the thread state; is this wrong?
If you want to keep Python state during your C thread, there are 4 lines to add to your function:
void *async_cb(void *dummy)
{
PyGILState_STATE gstate = PyGILState_Ensure();
Py_BEGIN_ALLOW_THREADS
(*callback_fn)();
(*callback_fn)();
Py_END_ALLOW_THREADS
PyGILState_Release(gstate);
pthread_exit(NULL);
}
|
msg116721 - (view) |
Author: Nikolaus Rath (nikratio) * |
Date: 2010-09-17 21:10 |
No, I am not saying that the behaviour of ctypes is wrong. It just happens to have some effects on threading.local that I think should be documented. That's why I reassigned this as a documentation bug.
Please reconsider closing this bug. I'm also happy to change the type to "Feature request".
As an aside: I think in most cases one uses ctypes to call functions that are already compiled, so changing the source is not an option.
|
msg116722 - (view) |
Author: Nikolaus Rath (nikratio) * |
Date: 2010-09-17 21:16 |
To be a bit more constructive, why not add something like this in paragraph to http://docs.python.org/library/ctypes.html#callback-functions:
"Note that if the callback function is called in a new thread that has been created outside of Python's control (i.e., by the foreign code that calls the callback), ctypes creates a new dummy Python thread on every invocation. That means that values stored with `threading.local` will *not* survive across different callbacks.
|
msg116725 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2010-09-17 22:13 |
This is not specific to ctypes.
Please read http://docs.python.org/c-api/init.html#thread-state-and-the-global-interpreter-lock
specially the paragraph that says "...when threads are created from C...".
Is it explicit enough? How would you change it?
|
msg116726 - (view) |
Author: Nikolaus Rath (nikratio) * |
Date: 2010-09-17 22:29 |
One point of ctypes is to save the user the trouble of having to create a full blown C extension, so I don't think it's reasonable to expect a ctypes user to have read the full C API documentation as well. Only a very small subset of the page that you gave is actually relevant for use with ctypes. Why not put this information where a ctypes user can find it easily?
|
msg116747 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2010-09-18 02:20 |
The suggestion in http://bugs.python.org/issue6627#msg116722 is a good one.
This a case where the user may legitimately not realise that threading.local is stored in the *temporary* state created by ctypes rather than in something more persistent inside the interpreter.
Since the ctypes state is per callback, it won't persist across calls, even when they're made from the same thread.
Suggested wording:
"""Note that if the callback function is called in a new thread that has been created outside of Python's control (i.e., by the foreign code that calls the callback), ctypes creates a new dummy Python thread on every invocation, including recreation of the thread local storage area. While this is correct for most purposes, it does mean that values stored with `threading.local` will *not* survive across different callbacks, even when those calls are made from the same C thread."""
|
msg117029 - (view) |
Author: Swapnil Talekar (swapnil) |
Date: 2010-09-21 04:38 |
Nick, the last statement,
"While this is correct for most purposes, it does mean that..."
can be simplified to,
"It means...".
I had to read it several times before I realized, there is no "not" after "does" :)
BTW, since this particular arrangement of having a temporary thread state during the callback is particularly useful for ctypes (I cannot imagine any other usecase) and also it sort-of breaks things, a potential feature request could be to have consistent thread state during the lifetime of a C thread. I don't have much idea how to do that or whether it is even possible? Would anyone like to give a thought?
|
msg117048 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2010-09-21 13:03 |
On Tue, Sep 21, 2010 at 2:39 PM, Swapnil Talekar <report@bugs.python.org> wrote:
> Swapnil Talekar <swapnil.st@gmail.com> added the comment:
> Nick, the last statement,
> "While this is correct for most purposes, it does mean that..."
> can be simplified to,
> "It means...".
> I had to read it several times before I realized, there is no "not" after "does" :)
The shorter version doesn't mean the same thing though - the ctypes
arrangement *really is* correct for most purposes. The only issue is
that threading.local won't persist, since the storage is blown away as
soon as the callback returns.
> BTW, since this particular arrangement of having a temporary thread state during the callback is particularly useful for ctypes (I cannot imagine any other usecase) and also it sort-of breaks things, a potential feature request could be to have consistent thread state during the lifetime of a C thread. I don't have much idea how to do that or whether it is even possible? Would anyone like to give a thought?
There's no easy way to make the thread state persist between calls, as
ctypes needs to destroy the thread state it creates at some point to
avoid a memory leak. Since it has no way of knowing when the
underlying C thread is no longer in use, it is forced to assume that
every call is going to be the last one from that thread and kill the
thread state.
|
msg185554 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2013-03-30 12:50 |
msg116747 suggests wording for a doc patch. Could this be applied and this issue closed? msg117029 talks about a potential feature request. Was this ever discussed?
|
msg207669 - (view) |
Author: Nikolaus Rath (nikratio) * |
Date: 2014-01-08 05:25 |
Here's a patch that (I think) incorporates all the comments. If someone could apply it, that would be great :-).
|
msg208521 - (view) |
Author: Nikolaus Rath (nikratio) * |
Date: 2014-01-20 04:44 |
(adding the documentation and ctypes experts from http://docs.python.org/devguide/experts.html to noisy list in the hope to get this moving again.)
|
msg208523 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-01-20 05:12 |
New changeset f4eade5df217 by Benjamin Peterson in branch '3.3':
document that a new Python thread context is created in ctypes callbacks (closes #6627)
http://hg.python.org/cpython/rev/f4eade5df217
New changeset 9cd2d7a3f9f2 by Benjamin Peterson in branch '2.7':
document that a new Python thread context is created in ctypes callbacks (closes #6627)
http://hg.python.org/cpython/rev/9cd2d7a3f9f2
New changeset fd647825475a by Benjamin Peterson in branch 'default':
merge 3.3 (#6627)
http://hg.python.org/cpython/rev/fd647825475a
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:51 | admin | set | github: 50876 |
2014-01-20 05:12:33 | python-dev | set | status: open -> closed
nosy:
+ python-dev messages:
+ msg208523
resolution: fixed stage: needs patch -> resolved |
2014-01-20 04:44:54 | nikratio | set | nosy:
+ georg.brandl, belopolsky, ezio.melotti, eric.araujo, meador.inge
messages:
+ msg208521 versions:
- Python 2.6, Python 3.1, Python 3.2, Python 3.5 |
2014-01-08 05:25:20 | nikratio | set | files:
+ issue6627.patch keywords:
+ patch messages:
+ msg207669
versions:
+ Python 3.1, Python 2.7, Python 3.2, Python 3.3, Python 3.4, Python 3.5 |
2013-07-24 13:08:32 | ronaldoussoren | set | nosy:
+ ronaldoussoren
|
2013-03-30 12:50:29 | BreamoreBoy | set | nosy:
+ BreamoreBoy messages:
+ msg185554
|
2010-09-21 13:03:21 | ncoghlan | set | messages:
+ msg117048 |
2010-09-21 04:39:01 | swapnil | set | messages:
+ msg117029 |
2010-09-18 02:20:36 | ncoghlan | set | status: closed -> open type: behavior -> enhancement messages:
+ msg116747
keywords:
+ easy resolution: works for me -> (no value) stage: resolved -> needs patch |
2010-09-17 22:29:15 | nikratio | set | messages:
+ msg116726 |
2010-09-17 22:13:20 | amaury.forgeotdarc | set | messages:
+ msg116725 |
2010-09-17 21:16:08 | nikratio | set | messages:
+ msg116722 |
2010-09-17 21:10:20 | nikratio | set | messages:
+ msg116721 |
2010-09-17 20:49:23 | amaury.forgeotdarc | set | status: open -> closed resolution: works for me messages:
+ msg116720
|
2010-09-17 18:58:30 | nikratio | set | resolution: not a bug -> (no value) |
2010-09-17 18:58:20 | nikratio | set | status: closed -> open
nosy:
+ docs@python messages:
+ msg116710
assignee: docs@python components:
+ Documentation, - Library (Lib) |
2010-09-17 15:28:45 | amaury.forgeotdarc | set | nosy:
+ amaury.forgeotdarc messages:
+ msg116673
|
2010-09-17 15:08:10 | ncoghlan | set | status: open -> closed resolution: not a bug messages:
+ msg116672
stage: resolved |
2010-09-17 14:43:43 | swapnil | set | nosy:
+ swapnil messages:
+ msg116671
|
2010-07-18 16:23:43 | anacrolix | set | nosy:
+ anacrolix
|
2010-02-16 15:06:18 | nikratio | set | messages:
+ msg99415 |
2010-02-16 15:03:10 | nikratio | set | files:
+ test.py |
2010-02-16 15:02:58 | nikratio | set | files:
+ lib.c |
2010-02-16 15:02:42 | nikratio | set | messages:
+ msg99414 |
2009-08-16 16:04:57 | verigak | set | nosy:
+ verigak
|
2009-08-07 16:20:02 | ncoghlan | set | nosy:
+ ncoghlan
|
2009-08-02 20:02:43 | nikratio | create | |