threading.local() does not work with C-created threads

[nikratio]

BPO	6627
Nosy	@birkenfeld, @ronaldoussoren, @amauryfa, @ncoghlan, @abalkin, @ezio-melotti, @merwok, @meadori, @anacrolix
Files	lib.c test.py issue6627.patch

^{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:

assignee = None
closed_at = <Date 2014-01-20.05:12:33.235>
created_at = <Date 2009-08-02.20:02:43.253>
labels = ['easy', 'type-feature', 'docs']
title = 'threading.local() does not work with C-created threads'
updated_at = <Date 2014-01-20.05:12:33.223>
user = 'https://bugs.python.org/nikratio'

bugs.python.org fields:

activity = <Date 2014-01-20.05:12:33.223>
actor = 'python-dev'
assignee = 'docs@python'
closed = True
closed_date = <Date 2014-01-20.05:12:33.235>
closer = 'python-dev'
components = ['Documentation']
creation = <Date 2009-08-02.20:02:43.253>
creator = 'nikratio'
dependencies = []
files = ['16232', '16233', '33361']
hgrepos = []
issue_num = 6627
keywords = ['patch', 'easy']
message_count = 19.0
messages = ['91198', '99414', '99415', '116671', '116672', '116673', '116710', '116720', '116721', '116722', '116725', '116726', '116747', '117029', '117048', '185554', '207669', '208521', '208523']
nosy_count = 15.0
nosy_names = ['georg.brandl', 'ronaldoussoren', 'amaury.forgeotdarc', 'ncoghlan', 'belopolsky', 'ezio.melotti', 'eric.araujo', 'nikratio', 'verigak', 'meador.inge', 'swapnil', 'anacrolix', 'docs@python', 'BreamoreBoy', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue6627'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

[nikratio]

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

[nikratio]

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

[nikratio]

Note also that if the from __future__ import is enabled, the program segfaults instead (separate bug)?

[swapnil]

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

[ncoghlan]

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.

[amauryfa]

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.

[nikratio]

@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().

[amauryfa]

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);
}

[nikratio]

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.

[nikratio]

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.

[amauryfa]

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?

[nikratio]

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?

[ncoghlan]

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."""

[swapnil]

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?

[ncoghlan]

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.

[BreamoreBoy]

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?

[nikratio]

Here's a patch that (I think) incorporates all the comments. If someone could apply it, that would be great :-).

[nikratio]

(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.)

[python-dev]

New changeset f4eade5df217 by Benjamin Peterson in branch '3.3':
document that a new Python thread context is created in ctypes callbacks (closes bpo-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 bpo-6627)
http://hg.python.org/cpython/rev/9cd2d7a3f9f2

New changeset fd647825475a by Benjamin Peterson in branch 'default':
merge 3.3 (bpo-6627)
http://hg.python.org/cpython/rev/fd647825475a