-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
PEP 788: Address feedback from the C API WG #4521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ZeroIntensity
wants to merge
10
commits into
python:main
Choose a base branch
from
ZeroIntensity:pep-788-third-round
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
a6718d0
Remove reference to old title.
ZeroIntensity 483557b
Use 0 for the failure value.
ZeroIntensity 9a12f44
Use 0 for the failure value in the examples.
ZeroIntensity f83e180
Rename PyInterpreterRef_AsInterpreter to PyInterpreterRef_GetInterpre…
ZeroIntensity b1818bc
Rename PyInterpreterRef_Main to PyUnstable_InterpreterRef_GetMain.
ZeroIntensity 47b6444
Rename PyInterpreterWeakRef_AsStrong to PyInterpreterWeakRef_Promote.
ZeroIntensity 754045e
Rename PyInterpreter[Weak]Ref_Get to PyInterpreter[Weak]Ref_FromCurrent.
ZeroIntensity 4ed9e36
Fix incorrect usage of PyInterpreterRef_FromCurrent().
ZeroIntensity 100d850
Rename PyUnstable_InterpreterRef_GetMain to PyUnstable_GetDefaultInte…
ZeroIntensity 4455670
Add a section describing PyUnstable_GetDefaultInterpreterRef's motiva…
ZeroIntensity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -325,10 +325,6 @@ in newer versions with the recent acceptance of :pep:`734`. | |
Rationale | ||
========= | ||
|
||
So, how do we address all of this? The best way seems to be starting from | ||
scratch and "reimagining" how to create, acquire and attach | ||
:term:`thread states <thread state>` in the C API. | ||
|
||
Preventing Interpreter Shutdown with Reference Counting | ||
------------------------------------------------------- | ||
|
||
|
@@ -391,6 +387,21 @@ The exact details of this deprecation aren't too clear. It's likely that | |
the usual five-year deprecation (as specificed by :pep:`387`) will be too | ||
short, so for now, these functions will have no specific removal date. | ||
|
||
Compatibility Shim for ``PyGILState_Ensure`` | ||
-------------------------------------------- | ||
|
||
This proposal comes with :c:func:`PyUnstable_GetDefaultInterpreterRef` as a | ||
compatibility hack for some users of :c:func:`PyGILState_Ensure`. It is a | ||
thread-safe way to acquire a strong reference to the main (or "default") | ||
interpreter. | ||
|
||
The main drawback to porting new code to :c:func:`PyThreadState_Ensure` is that | ||
it isn't a drop-in replacement for :c:func:`!PyGILState_Ensure`, as it needs | ||
an interpreter reference argument. In some large applications, refactoring to | ||
use a :c:type:`PyInterpreterRef` everywhere might be tricky; so, this function | ||
acts as a silver bullet for users who explicitly want to disallow support for | ||
subinterpreters. | ||
|
||
Specification | ||
============= | ||
|
||
|
@@ -405,14 +416,14 @@ around the same time when :class:`threading.Thread` objects are joined, but | |
note that this *is not* the same as joining the thread; the interpreter will | ||
only wait until the reference count is zero, and then proceed. | ||
After the reference count has reached zero, threads can no longer prevent the | ||
interpreter from shutting down (thus :c:func:`PyInterpreterRef_Get` and | ||
:c:func:`PyInterpreterWeakRef_AsStrong` will fail). | ||
interpreter from shutting down (thus :c:func:`PyInterpreterRef_FromCurrent` and | ||
:c:func:`PyInterpreterWeakRef_Promote` will fail). | ||
|
||
A weak reference to an interpreter won't prevent it from finalizing, and can | ||
be safely accessed after the interpreter no longer supports creating strong | ||
references, and even after the interpreter-state has been deleted. Deletion | ||
and duplication of the weak reference will always be allowed, but promotion | ||
(:c:func:`PyInterpreterWeakRef_AsStrong`) will always fail after the | ||
(:c:func:`PyInterpreterWeakRef_Promote`) will always fail after the | ||
interpreter reaches a point where strong references have been waited on. | ||
|
||
Strong Interpreter References | ||
|
@@ -421,43 +432,43 @@ Strong Interpreter References | |
.. c:type:: PyInterpreterRef | ||
|
||
An opaque, strong reference to an interpreter. | ||
|
||
The interpreter will wait until a strong reference has been released | ||
before shutting down. | ||
|
||
This type is guaranteed to be pointer-sized. | ||
|
||
.. c:function:: int PyInterpreterRef_Get(PyInterpreterRef *ref) | ||
.. c:function:: PyInterpreterRef PyInterpreterRef_FromCurrent(void) | ||
|
||
Acquire a strong reference to the current interpreter. | ||
|
||
On success, this function returns ``0`` and sets *ref* | ||
to a strong reference to the interpreter, and returns ``-1`` | ||
with an exception set on failure. | ||
On success, this function returns a strong reference to the current | ||
interpreter, and returns ``0`` with an exception set on failure. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer to "enforce error checking" by returning PyInterpreterRef ref;
if (PyInterpreterRef_Get(&ref) < 0) {
return NULL;
} But if others prefer checking if ref is zero, I would be fine with that API as well. |
||
|
||
Failure typically indicates that the interpreter has | ||
already finished waiting on strong references. | ||
Failure typically indicates that the interpreter has already finished | ||
waiting on strong references. | ||
|
||
The caller must hold an :term:`attached thread state`. | ||
|
||
.. c:function:: int PyInterpreterRef_Main(PyInterpreterRef *ref) | ||
.. c:function:: PyInterpreterRef PyUnstable_GetDefaultInterpreterRef(PyInterpreterRef *ref) | ||
|
||
Acquire a strong reference to the main interpreter. | ||
|
||
This function only exists for special cases where a specific interpreter | ||
can't be saved. Prefer safely acquiring a reference through | ||
:c:func:`PyInterpreterRef_Get` whenever possible. | ||
:c:func:`PyInterpreterRef_FromCurrent` whenever possible. | ||
|
||
On success, this function will return ``0`` and set *ref* to a strong | ||
reference, and on failure, this function will return ``-1``. | ||
On success, this function returns a strong reference to the main | ||
interpreter, and returns ``0`` without an exception set on failure. | ||
|
||
Failure typically indicates that the main interpreter has already finished | ||
waiting on its reference count. | ||
|
||
The caller does not need to hold an :term:`attached thread state`. | ||
|
||
.. c:function:: PyInterpreterState *PyInterpreterRef_AsInterpreter(PyInterpreterRef ref) | ||
.. c:function:: PyInterpreterState *PyInterpreterRef_GetInterpreter(PyInterpreterRef ref) | ||
|
||
Return the interpreter denoted by *ref*. | ||
Return the :c:type:`PyInterpreterState` pointer denoted by *ref*. | ||
|
||
This function cannot fail, and the caller doesn't need to hold an | ||
:term:`attached thread state`. | ||
|
@@ -466,8 +477,10 @@ Strong Interpreter References | |
|
||
Duplicate a strong reference to an interpreter. | ||
|
||
This function cannot fail, and the caller doesn't need to hold an | ||
:term:`attached thread state`. | ||
On success, this function returns a strong reference to the interpreter | ||
denoted by *ref*, and returns ``0`` without an exception set on failure. | ||
|
||
The caller does not need to hold an :term:`attached thread state`. | ||
|
||
.. c:function:: void PyInterpreterRef_Close(PyInterpreterRef ref) | ||
|
||
|
@@ -483,40 +496,45 @@ Weak Interpreter References | |
.. c:type:: PyInterpreterWeakRef | ||
|
||
An opaque, weak reference to an interpreter. | ||
|
||
The interpreter will *not* wait for the reference to be | ||
released before shutting down. | ||
|
||
This type is guaranteed to be pointer-sized. | ||
|
||
.. c:function:: int PyInterpreterWeakRef_Get(PyInterpreterWeakRef *wref) | ||
.. c:function:: int PyInterpreterWeakRef_FromCurrent(PyInterpreterWeakRef *wref) | ||
|
||
Acquire a weak reference to the current interpreter. | ||
|
||
This function is generally meant to be used in tandem with | ||
:c:func:`PyInterpreterWeakRef_AsStrong`. | ||
:c:func:`PyInterpreterWeakRef_Promote`. | ||
|
||
On success, this function returns ``0`` and sets *wref* to a | ||
weak reference to the interpreter, and returns ``-1`` with an exception | ||
set on failure. | ||
On success, this function returns a weak reference to the current | ||
interpreter, and returns ``0`` with an exception set on failure. | ||
|
||
The caller must hold an :term:`attached thread state`. | ||
|
||
.. c:function:: PyInterpreterWeakRef PyInterpreterWeakRef_Dup(PyInterpreterWeakRef wref) | ||
|
||
Duplicate a weak reference to an interpreter. | ||
|
||
On success, this function returns a non-zero weak reference to the | ||
interpreter denoted by *wref*, and returns ``0`` without an exception set | ||
on failure. | ||
|
||
This function cannot fail, and the caller doesn't need to hold an | ||
:term:`attached thread state`. | ||
|
||
.. c:function:: int PyInterpreterWeakRef_AsStrong(PyInterpreterWeakRef wref, PyInterpreterRef *ref) | ||
.. c:function:: PyInterpreterRef PyInterpreterWeakRef_Promote(PyInterpreterWeakRef wref) | ||
vstinner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Acquire a strong reference to an interpreter through a weak reference. | ||
|
||
On success, this function returns ``0`` and sets *ref* to a strong | ||
reference to the interpreter denoted by *wref*. | ||
On success, this function returns a strong reference to the interpreter | ||
denoted by *wref*. The weak reference is still valid after calling this | ||
function. | ||
|
||
If the interpreter no longer exists or has already finished waiting | ||
for its reference count to reach zero, then this function returns ``-1`` | ||
for its reference count to reach zero, then this function returns ``0`` | ||
without an exception set. | ||
|
||
This function is not safe to call in a re-entrant signal handler. | ||
|
@@ -648,16 +666,16 @@ With this PEP, you'd implement it like this: | |
PyObject *file, | ||
const char *text) | ||
{ | ||
PyInterpreterRef ref; | ||
if (PyInterpreterWeakRef_AsStrong(wref, &ref) < 0) { | ||
PyInterpreterRef ref = PyInterpreterWeakRef_Promote(wref); | ||
if (ref == 0) { | ||
/* Python interpreter has shut down */ | ||
return -1; | ||
} | ||
|
||
PyThreadRef thread_ref; | ||
if (PyThreadState_Ensure(ref, &thread_ref) < 0) { | ||
PyInterpreterRef_Close(ref); | ||
puts("Out of memory.\n", stderr); | ||
fputs("Cannot call Python.\n", stderr); | ||
return -1; | ||
} | ||
|
||
|
@@ -693,8 +711,8 @@ held. Any future finalizer that wanted to acquire the lock would be deadlocked! | |
my_critical_operation(PyObject *self, PyObject *unused) | ||
{ | ||
assert(PyThreadState_GetUnchecked() != NULL); | ||
PyInterpreterRef ref; | ||
if (PyInterpreterRef_Get(&ref) < 0) { | ||
PyInterpreterRef ref = PyInterpreterRef_FromCurrent(); | ||
if (ref == 0) { | ||
/* Python interpreter has shut down */ | ||
return NULL; | ||
} | ||
|
@@ -776,8 +794,8 @@ This is the same code, rewritten to use the new functions: | |
PyThread_handle_t handle; | ||
PyThead_indent_t indent; | ||
|
||
PyInterpreterRef ref; | ||
if (PyInterpreterRef_Get(&ref) < 0) { | ||
PyInterpreterRef ref = PyInterpreterRef_FromCurrent(); | ||
if (ref == 0) { | ||
return NULL; | ||
} | ||
|
||
|
@@ -797,7 +815,8 @@ Example: A Daemon Thread | |
|
||
With this PEP, daemon threads are very similar to how non-Python threads work | ||
in the C API today. After calling :c:func:`PyThreadState_Ensure`, simply | ||
release the interpreter reference, allowing the interpreter to shut down. | ||
release the interpreter reference to allow the interpreter to shut down (and | ||
hang the current thread forever). | ||
|
||
.. code-block:: c | ||
|
||
|
@@ -826,8 +845,8 @@ release the interpreter reference, allowing the interpreter to shut down. | |
PyThread_handle_t handle; | ||
PyThead_indent_t indent; | ||
|
||
PyInterpreterRef ref; | ||
if (PyInterpreterRef_Get(&ref) < 0) { | ||
PyInterpreterRef ref = PyInterpreterRef_FromCurrent(); | ||
if (ref == 0) { | ||
return NULL; | ||
} | ||
|
||
|
@@ -856,8 +875,8 @@ deadlock the interpreter if it's not released. | |
{ | ||
ThreadData *data = (ThreadData *)arg; | ||
PyInterpreterWeakRef wref = data->wref; | ||
PyInterpreterRef ref; | ||
if (PyInterpreterWeakRef_AsStrong(wref, &ref) < 0) { | ||
PyInterpreterRef ref = PyInterpreterWeakRef_Promote(wref); | ||
if (ref == 0) { | ||
fputs("Python has shut down!\n", stderr); | ||
return -1; | ||
} | ||
|
@@ -885,8 +904,8 @@ deadlock the interpreter if it's not released. | |
PyErr_NoMemory(); | ||
return NULL; | ||
} | ||
PyInterpreterWeakRef wref; | ||
if (PyInterpreterWeakRef_Get(&wref) < 0) { | ||
PyInterpreterWeakRef wref = PyInterpreterWeakRef_FromCurrent(); | ||
if (wref == 0) { | ||
PyMem_RawFree(tdata); | ||
return NULL; | ||
} | ||
|
@@ -902,7 +921,7 @@ Example: Calling Python Without a Callback Parameter | |
There are a few cases where callback functions don't take a callback parameter | ||
(``void *arg``), so it's impossible to acquire a reference to any specific | ||
interpreter. The solution to this problem is to acquire a reference to the main | ||
interpreter through :c:func:`PyInterpreterRef_Main`. | ||
interpreter through :c:func:`PyUnstable_GetDefaultInterpreterRef`. | ||
|
||
But wait, won't that break with subinterpreters, per | ||
:ref:`pep-788-subinterpreters-gilstate`? Fortunately, since the callback has | ||
|
@@ -915,9 +934,9 @@ interpreter here. | |
static void | ||
call_python(void) | ||
{ | ||
PyInterpreterRef ref; | ||
if (PyInterpreterRef_Main(&ref) < 0) { | ||
fputs("Python has shut down!", stderr); | ||
PyInterpreterRef ref = PyUnstable_GetDefaultInterpreterRef(); | ||
if (ref == 0) { | ||
fputs("Python has shut down.", stderr); | ||
return; | ||
} | ||
|
||
|
@@ -1009,7 +1028,7 @@ of requiring less magic: | |
the non-Python thread gets a chance to attach. The problem with using an | ||
interpreter ID is that the reference count has to be "invisible"; it | ||
must be tracked elsewhere in the interpreter, likely being *more* | ||
complex than :c:func:`PyInterpreterRef_Get`. There's also a lack | ||
complex than :c:func:`PyInterpreterRef_FromCurrent`. There's also a lack | ||
of intuition that a standalone integer could have such a thing as | ||
a reference count. | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer
PyInterpreterRef_Get()
andPyInterpreterWeakRef_Get()
which are consistent with existingPyThreadState_Get()
andPyInterpreterState_Get()
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me too, but please bring this up on the C API WG discussion. I'm just following suggestions from Steve and Petr here.