[issue15751] Support subinterpreters in the GIL state API

[Maciej Szulik]

Change by Maciej Szulik :


--
nosy: +maciej.szulik

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Paulo Henrique Silva]

Change by Paulo Henrique Silva :


--
nosy: +phsilva

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Eric Snow]

Change by Eric Snow :


--
versions: +Python 3.8, Python 3.9 -Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Roundup Robot]

Roundup Robot added the comment:

New changeset e590c632c9fa by Victor Stinner in branch 'default':
Add more checks on the GIL
https://hg.python.org/cpython/rev/e590c632c9fa

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Eric Snow]

Changes by Eric Snow ericsnowcurren...@gmail.com:


--
versions: +Python 3.6 -Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Roundup Robot]

Roundup Robot added the comment:

New changeset 5d078b0bae75 by Victor Stinner in branch 'default':
Issue #19787: PyThread_set_key_value() now always set the value
http://hg.python.org/cpython/rev/5d078b0bae75

--
nosy: +python-dev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[STINNER Victor]

STINNER Victor added the comment:

 FYI I fixed a weird behaviour of the PyThread_set_key_value() function.  The 
 change has indirectly on impact on test.support.run_in_subinterp():

The impact was a bug. I reverted my changeset and pushed a new changeset which 
leaves _PyGILState_NoteThreadState() unchanged.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[STINNER Victor]

STINNER Victor added the comment:

FYI I fixed a weird behaviour of the PyThread_set_key_value() function. The 
change has indirectly on impact on test.support.run_in_subinterp():
http://bugs.python.org/issue19787#msg206015

So my change might have an effect on this issue.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[STINNER Victor]

STINNER Victor added the comment:

They are now two issues (#10915 and this one) with many messages. Both issues 
are open. Can someone please make a summary? How can we fix the 
GIL/subinterpreter issue?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Antoine Pitrou]

Antoine Pitrou added the comment:

I wanted to set issue10915 as duplicate but there is actually a tentative patch 
there. Unfortunately the discussions are now split apart...

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


--
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Eric Snow]

Changes by Eric Snow ericsnowcurren...@gmail.com:


--
nosy: +eric.snow

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Jesús Cea Avión]

Changes by Jesús Cea Avión j...@jcea.es:


--
nosy: +jcea

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Antoine Pitrou]

Antoine Pitrou added the comment:

Le mercredi 29 août 2012 à 02:19 +, Nick Coghlan a écrit :
 However, it *doesn't* work (at least, not easily) if the extension
 itself wants to call back into an interpreter other than the one
 already associated with the current thread:
 
/* Embedding application (GIL always unlocked) */
gilstate = PyGILState_EnsureEx(interp_A);
  /* Python code and extension code happens */
/* Callback needs to reach back into a specific interpreter */
cb_gilstate = PyGILState_EnsureEx(interp_B);
  /* Callback runs */
PyGILState_Release(cb_gilstate);
  /* Python code and extension code unwinds */
PyGILState_Release(gilstate);
 
 Does that second call to EnsureEx fail?

What would it fail? It should simply change the thread state to
interp_B's thread state for the current thread. Then the nested
PyGILState_Release changes the thread state back to its old value.

(of course, I don't understand how an extension running from a given
interpreter would have access to another interpreter's callback, but
regardless, it's technically not a problem)

 If it succeeds, how does the client know which interpreter to use for
 the PyGILState_Release call? It could be made to work if
 PyGILState_STATE was changed from an enum to a struct that included in
 interpreter state pointer, or if EnsureEx returned a different type
 and was paired up with a new ReleaseEx pointer.

Making PyGILState_STATE a struct sounds reasonable to me.

 However, that's starting to get very messy compared to a separate
 SwitchInterpreter call:

Why messy?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Nick Coghlan]

Nick Coghlan added the comment:

Yeah, I eventually came around to agreeing that it's better to stick with the 
Ensure/Release model. SwitchInterpreter gets messy when it comes to managing 
the lifecycle of temporary thread states.

So an EnsureEx/ReleaseEx pair with a new struct that includes both the GIL 
state info and either the previous thread state or an interpreter pointer 
sounds good to me.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Graham Dumpleton]

Graham Dumpleton added the comment:

If PyGILState_STATE is a struct, what happens if someone naively does:

PyGILState_Release(PyGILState_UNLOCKED)

I know they shouldn't, but I actually do this in mod_wsgi in one spot as it is 
otherwise a pain to carry around the state when I know for sure if was unlocked 
before the PyGILState_Ensure().

Or can PyGILState_UNLOCKED map to some a global struct instance with certain 
state in it that represents that without problem.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Mark Hammond]

Mark Hammond added the comment:

The GIL state api was mainly interested in the case of a thread which has 
(possibly) never been seen before calling into Python.  IIUC, the proposal here 
is so that a thread that *has* been seen before can be associated with a 
thread-state specified by the embedding application (and the degenerate case 
would be to assume the thread hasn't been seen, and as such should get the 
default interpreter)

If that isn't too wide of the mark, I agree it sounds workable and worthwhile.

--
nosy: +mhammond

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Mark Hammond]

Mark Hammond added the comment:

To clarify, I wrote:

 can be associated with a thread-state specified by the 
 embedding application 

Where I meant to say:

Can be associated with an interpreter state and corresponding thread-state ...

Or something like that - it's been a while since I've looked at that code.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Nick Coghlan]

Nick Coghlan added the comment:

Thinking about it, I believe there still needs to be a concept of an active 
thread state TLS key in order to deal with Graham's original problem. 
Specifically, if PyGILState_EnsureEx is used to associate the thread with a 
particular interpreter, then subsequent calls to PyGILState_Ensure from *that 
thread* should get the explicitly associated interpreter, rather than the main 
interpreter.

Then, the lookup sequence for interpreter=NULL would be:

1. Check the active TLS key, if set, use that thread state
2. If not set, look up the main interpreter's TLS key. If set, also set that on 
the active TLS key and use that thread state.
3. If still not set, create a new thread state on the main interpreter, set it 
for both the active TLS and the main interpreter's TLS key and use that thread 
state

A similar approach almost works when requesting a specific interpreter, but 
where that goes wrong is when the active TLS key is *already set*. You can't 
just overwrite it, because that will cause problems for subsequent 
PyGIL_Release calls. You could just make it an error, but I think Graham's 
original idea makes it possible to do better than that.

Specifically, a PyGILState_SwitchInterpreter API could focus solely on the 
manipulation of the active thread state TLS key entry. The sequence of 
commands in mod_wsgi would then look like:

old_interp = PyGILState_SwitchInterpreter(target_interp);
old_gil = PyGILState_Ensure();
/* Call into Python using target_interp */
PyGILState_Release(old_gil);
PyGILState_SwitchInterpreter(old_interp); /* May not be needed in the mod_wsgi 
case, since it knows it is making the outermost call into the PyGILState_* APIs 
*/

All of the other elements of Antoine's proposal (i.e. the per-interpreter TLS 
key entries) would still be needed, it's just that the API for associating a 
thread with an interpreter would remain separated from that of associating the 
thread with a particular thread state.

The big advantage of doing it this way is that it will nest properly, whereas 
PyGILState_EnsureEx would need a more complicated API to correctly report both 
the old interpreter state and the old GIL state within that interpreter.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Antoine Pitrou]

Antoine Pitrou added the comment:

Le mardi 28 août 2012 à 14:12 +, Nick Coghlan a écrit :
 old_interp = PyGILState_SwitchInterpreter(target_interp);
 old_gil = PyGILState_Ensure();
 /* Call into Python using target_interp */
 PyGILState_Release(old_gil);
 PyGILState_SwitchInterpreter(old_interp); /* May not be needed in the 
 mod_wsgi case, since it knows it is making the outermost call into the 
 PyGILState_* APIs */

Why wouldn't it be simply written:

old_gil = PyGILState_EnsureEx(target_interp);
/* Call into Python using target_interp */
PyGILState_Release(old_gil);

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Graham Dumpleton]

Graham Dumpleton added the comment:

Sorry, Mark. Is not for associating thread state specified by embedding 
application. In simple terms it is exactly like existing PyGILState_Ensure() in 
that caller doesn't have a thread state, whether it has already been created. 
Only difference is to allow that simplified API to work against a sub 
interpreter.

Nick, I previously said:

In the case of the latter, if a Python thread is created in a specific sub 
interpreter, it should automatically designate for that thread that that is its 
interpreter context, so if it calls out and does the Release/Ensure dance, that 
it goes back into the same sub interpreter.

So yes to your:

Thinking about it, I believe there still needs to be a concept of an active 
thread state TLS key in order to deal with Graham's original problem. 
Specifically, if PyGILState_EnsureEx is used to associate the thread with a 
particular interpreter, then subsequent calls to PyGILState_Ensure from *that 
thread* should get the explicitly associated interpreter, rather than the main 
interpreter.

My example was more to do with a thread created in Python then calling out and 
back in, but same deal as foreign thread calling in, out and back in.

Antoine, yes, can possibly can be simplified to that. The original idea of a 
switch interpreter function was suggested on basis that PyGILState_Ensure would 
not be modified or extended version of function created. Rolling an implicit 
switch interpreter into PyGILState_EnsureEx when argument is different to the 
current may serve same purpose.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Nick Coghlan]

Nick Coghlan added the comment:

The reason I'm proposing going back to the original SwitchInterpreter idea is 
because the EnsureEx idea doesn't nest cleanly - if your thread is already 
associated with interpreter A, you can't readily call into interpeter B, 
because the API provides no way to correctly restore the associated interpreter 
back to interpreter A when you're done.

EnsureEx works fine if extension modules are not aware of multiple interpreters:

   /* Embedding application (GIL always unlocked) */
   gilstate = PyGILState_EnsureEx(interp_A);
 /* Python code and extension code happens */
   /* Callback needs to reach back into Python */
   cb_gilstate = PyGILState_Ensure();
 /* Callback runs */
   PyGILState_Release(cb_gilstate);
 /* Python code and extension code unwinds */
   PyGILState_Release(gilstate);

However, it *doesn't* work (at least, not easily) if the extension itself wants 
to call back into an interpreter other than the one already associated with the 
current thread:

   /* Embedding application (GIL always unlocked) */
   gilstate = PyGILState_EnsureEx(interp_A);
 /* Python code and extension code happens */
   /* Callback needs to reach back into a specific interpreter */
   cb_gilstate = PyGILState_EnsureEx(interp_B);
 /* Callback runs */
   PyGILState_Release(cb_gilstate);
 /* Python code and extension code unwinds */
   PyGILState_Release(gilstate);

Does that second call to EnsureEx fail? If it succeeds, how does the client 
know which interpreter to use for the PyGILState_Release call? It could be made 
to work if PyGILState_STATE was changed from an enum to a struct that included 
in interpreter state pointer, or if EnsureEx returned a different type and was 
paired up with a new ReleaseEx pointer.

However, that's starting to get very messy compared to a separate 
SwitchInterpreter call:

   /* Embedding application (GIL always unlocked) */
   old_interp = PyGILState_SwitchInterpreter(interp_A);
   /* autoTLSkey now refers to a thread state for interpreter A */
   gilstate = PyGILState_Ensure();
 /* Python code and extension code happens */
   /* Callback needs to reach back into Python */
   pre_cb_interp = PyGILState_SwitchInterpreter(interp_B);
   /* autoTLSkey now refers to a thread state for interpreter B */
   cb_gilstate = PyGILState_Ensure();
 /* Callback runs */
   PyGILState_Release(cb_gilstate);
   PyGILState_SwitchInterpreter(pre_cb_interp);
   /* autoTLSkey again refers to a thread state for interpreter A */
 /* Python code and extension code unwinds */
   PyGILState_Release(gilstate);
   PyGILState_SwitchInterpreter(old_interp);

And yes, I'm pondering ways that this could be used to implement Rust-style 
channels for communication between interpreters without needing to copy data, 
by using this API to create proxy interfaces for accessing an object owned by 
another subinterpreter.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Graham Dumpleton]

Graham Dumpleton added the comment:

Nick. Valid point.

I guess I hadn't been thinking about case of one thread calling out of one 
interpreter and then into another, as I don't do it in mod_wsgi and even I 
regard doing that as partly evil.

Does that mean this switch interpreter call somehow gets used in the 
Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Nick Coghlan]

Nick Coghlan added the comment:

No, those wouldn't change at all. However, after thinking about it further, I'm 
leaning back towards the option of an EnsureEx/ReleaseEx pair that allows the 
previous interpreter to be reported and restored along with the GIL state. I'd 
also like to make the state an opaque structure that isn't exposed in the 
limited API. Something along the lines of:

typedef struct _gilinfo {
PyGILState_STATE  lock_state;
PyInterpreter *active;
} PyGILState_INFO;

int /* Allow returning an error code */
PyGILState_EnsureEx(PyInterpreterState *target,
PyGILState_INFO*prev);

void
PyGILState_ReleaseEx(PyGILState_INFO *prev);

This allows various issues related to implicitly creating thread states to be 
handled the same way they are now, where PyThreadState_New will create a 
persistent thread state, while PyGILState_EnsureEx will either use a 
preexisting thread state for the requested interpreter (if it exists), or 
implicitly create one. Implicitly created thread states would be destroyed by 
the corresponding call to ReleaseEx.

To make this work, I believe all that should be needed is for:

1. PyInterpreterState updated to include a per-interpreter TLS key
2. _PyGILState_NoteThreadState would update both the autoTLSkey entry and the 
per-interpreter key entry
3. PyGILState_EnsureEx added to support switching the autoTLSkey entry to point 
to a different thread state (creating it if needed)
4. PyGILState_Ensure updated to map to PyGILState_EnsureEx(NULL) and to extract 
the lock state from the info structure
5. PyGILState_Release updated to populate the info structure with the active 
interpreter and the passed in previous GIL state before calling 
PyGILState_ReleaseEx
5. PyThreadState_Delete and PyThreadState_DeleteCurrent updated to also clobber 
the per-interpreter TLS key entry

PyThreadState_New already calls _PyGILState_NoteThreadState implicitly, so 
Python created threads (and embedded threads with an explicitly created thread 
state) should do the right thing automatically.

Making the INFO struct an opaque token also means it can be expanded to cover 
any future needs that arise.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Graham Dumpleton]

Graham Dumpleton added the comment:

So you are saying that as user of this I am meant to call it as:

PyGILState_INFO info;

PyGILState_EnsureEx(interp, info);
...
PyGILState_ReleaseEx(info);

What is the potential error code from PyGILState_EnsureEx() considering that 
right now for PyGILState_Ensure() it is a value passed back into 
PyGILState_Release().

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Nick Coghlan]

Nick Coghlan added the comment:

Currently, both PyGILState_Ensure and PyGILState_Release just call 
PyFatal_Error if anything goes wrong. With the *Ex versions, it would be 
possible to replace those fatal errors with more conventional error handling.

For Ensure, the likely error is failing to create the thread state.

For Release, the likely error is calling it with the system in the wrong state.

I think the current API reflects a difference between an extension mindset and 
an embedding mindset. If those calls go wrong in an extension context, killing 
the entire application is likely the only thing you can reasonably do. In an 
embedded context, though, the embedding application likely has other options 
(such as reinitialising the entire Python interpreter).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Nick Coghlan]

Nick Coghlan added the comment:

And the current autoTLSkey could move into the interpreter state object? I 
like it - that's a lot more flexible than the approach I was thinking of.

--
title: Add PyGILState_SwitchInterpreter - Support subinterpreters in the GIL 
state API

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15751] Support subinterpreters in the GIL state API

[Graham Dumpleton]

Graham Dumpleton added the comment:

It is past my bed time and not thinking straight, but I believe Antoine is 
aligned with what I had in mind, as need multiple thread states per OS thread 
where each is associated with separate interpreter.

My main reason for allowing NULL to EnsureEX rather than requiring 
main_interpreter to be explicitly passed, is that way way back in time, my 
recollection is that getting access to the main interpreter pointer was a pain 
as you had to iterate over the list of interpreters and assume it was the last 
one due to it being created first. I don't remember there being a special 
global variable or function for getting a pointer to the main interpreter. This 
may well have changed since and there is an easier way do let me know. So saw 
it as a convenience.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15751
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com