python / cpython

The Python programming language
https://www.python.org
Other
63.51k stars 30.42k forks source link

[subinterpreters] Make the PyGILState API compatible with subinterpreters #59956

Open ncoghlan opened 12 years ago

ncoghlan commented 12 years ago
BPO 15751
Nosy @mhammond, @jcea, @ronaldoussoren, @ncoghlan, @pitrou, @vstinner, @phsilva, @asvetlov, @ericsnowcurrently, @soltysh, @seberg, @erlend-aasland, @Felk, @h-vetinari

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: ```python assignee = None closed_at = None created_at = labels = ['expert-subinterpreters', 'expert-C-API', 'type-feature', '3.10'] title = '[subinterpreters] Make the PyGILState API compatible with subinterpreters' updated_at = user = 'https://github.com/ncoghlan' ``` bugs.python.org fields: ```python activity = actor = 'vstinner' assignee = 'none' closed = False closed_date = None closer = None components = ['C API', 'Subinterpreters'] creation = creator = 'ncoghlan' dependencies = [] files = [] hgrepos = [] issue_num = 15751 keywords = [] message_count = 49.0 messages = ['168742', '168759', '168764', '168766', '168768', '168769', '168771', '168772', '168776', '168779', '168780', '168787', '168789', '168790', '168792', '168812', '168815', '168816', '168818', '168832', '169008', '169010', '169013', '169014', '169278', '169280', '169284', '169286', '169310', '169329', '169330', '169332', '169333', '169334', '169352', '169354', '169359', '198115', '206016', '206017', '206042', '206048', '261779', '368885', '368904', '368906', '375134', '380325', '409995'] nosy_count = 17.0 nosy_names = ['mhammond', 'jcea', 'ronaldoussoren', 'ncoghlan', 'pitrou', 'vstinner', 'phsilva', 'grahamd', 'asvetlov', 'python-dev', 'eric.snow', 'maciej.szulik', 'seberg', 'erlendaasland', 'felk', 'h-vetinari', 'gabrielhae'] pr_nums = [] priority = 'normal' resolution = None stage = 'needs patch' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue15751' versions = ['Python 3.10'] ```

Linked PRs

ericsnowcurrently commented 1 year ago

FYI, I have a PR up that partially fixes the incompatibility: gh-101431.

It makes it so the GILState thread state and the "current" thread state (from PyThreadState_Swap()) don't get out of sync. So now PyThreadState_GET() == PyGILState_GetThisThreadState() will always be true (while the GIL is held).

The PR does not address autoInterpreterState, which determines which interpreter is used to create a new thread state.

yqs112358 commented 1 year ago

It is known that Python3.12 has supported per-interpreter GIL. Considering that running different sub-interpreters in parallel in multiple threads becomes a reality, it is a necessity to make the PyGILState API compatible.

Hope this will be fixed later in 3.12.x. ❤ Or, is there any instructions to teach us how to safely call PyGILState_Ensure() and PyGILState_Release() when using sub-interpreters in Non-Python created threads?

ericsnowcurrently commented 1 year ago

Considering that running different sub-interpreters in parallel in multiple threads becomes a reality, it is a necessity to make the PyGILState API compatible.

Just to be clear, this issue describes two distinct but related concerns:

[^1]: it always uses the main interpreter

Presumably you are talking about the bug part.

Hope this will be fixed later in 3.12.x.

The bug part should be fixed in 3.12, via gh-101431[^2] (and mostly tested via gh-101625). There should no longer be any crashes or inconsistent state when using the GILState API with subinterpreters. (The feature part of this issue is still unresolved.)

[^2]: Note that the main objective of gh-101431 was to preserve the existing behavior of the GILState API, which was impacted by various runtime changes needed for a per-interpreter GIL. However, my understanding was that it also solved the bug part of this issue.

If the PR didn't actually solve the problem then we'll see if we can fix it in 3.12. However, a fix would have to wait until 3.13 if it requires adding new API.

Is there a specific case that isn't working for you on 3.12? If so, please provide an example that reproduces the failure.

Or, is there any instructions to teach us how to safely call PyGILState_Ensure() and PyGILState_Release() when using sub-interpreters in Non-Python created threads?

It shouldn't need any special treatment. Use PyGILState_Ensure() like normal. You don't get to pick which interpreter state any new thread state targets though. That's the feature part of this issue that still needs to be solved (in 3.13).

pitrou commented 1 year ago

Not speaking for the OP, but if PyGILState_Ensure() always selects the main interpreter it means that any extension using it will not be compatible with subinterpreters (unless the piece of code that's guarded by PyGILState_Ensure is stateless enough that it doesn't matter which interpreter it executes in).

For example, if an extension gives access to a database and allows writing user-defined functions (UDFs) in Python, UDFs would then be always executed in the main interpreter which is not necessarily expected.

What would be needed is an additional API such as:

PyGILState_STATE_EX PyGILState_EnsureWithInterpreter(PyInterpreterState*);
void PyGILState_ReleaseWithInterpreter(PyGILState_STATE_EX, PyInterpreterState*);

(PyGILState_STATE_EX might or might not be the same as PyGILState_STATE)

... with one open question being: what happens if the given interpreter was destroyed in the meantime? (should there instead be some kind of interpreter id pointing to a hash table of interpreters?)