Skip to content
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

gh-109549: Add new states to PyThreadState to support PEP 703 #109915

Merged
merged 3 commits into from
Oct 5, 2023

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury commented Sep 26, 2023

This adds a new field 'state' to PyThreadState that can take on one of three values: _Py_THREAD_ATTACHED, _Py_THREAD_DETACHED, or _Py_THREAD_GC. The "attached" and "detached" states correspond closely to closely to acquiring and releasing the GIL. The "gc" state is current unused, but will be used to implement stop-the-world GC for --disable-gil builds in the near future.

This adds a new field 'state' to PyThreadState that can take on one of
three values: _Py_THREAD_ATTACHED, _Py_THREAD_DETACHED, and
_Py_THREAD_GC. The "attached" and "detached" states correspond closely
to closely to acquiring and releasing the GIL. The "gc" state is
current unused, but will be used to implement stop-the-world GC for
`--disable-gil` builds in the near future.
@colesbury
Copy link
Contributor Author

!buildbot nogil

@bedevere-bot
Copy link

The regex 'nogil' did not match any buildbot builder.Is the requested builder in the list of stable builders?

@colesbury
Copy link
Contributor Author

!buildbot GIL

@bedevere-bot
Copy link

The regex 'GIL' did not match any buildbot builder.Is the requested builder in the list of stable builders?

@@ -2095,12 +2092,11 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
*tstate_p = NULL;

/* Oops, it didn't work. Undo it all. */
PyErr_PrintEx(0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the PyErr_PrintEx(0) call, which never printed anything (failures are reported via PyStatus) and was also unsafe to call in some cases when the current thread did not hold the GIL.

@colesbury
Copy link
Contributor Author

@ericsnowcurrently, would you please review this when you get a chance?

Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of comments that I'd like preserved, but otherwise LGTM.

Python/ceval_gil.c Show resolved Hide resolved
Python/pystate.c Show resolved Hide resolved
Python/pystate.c Show resolved Hide resolved
Python/pystate.c Show resolved Hide resolved
@colesbury
Copy link
Contributor Author

I've merged the 'main' branch into this PR to resolve merge conflicts.

@ericsnowcurrently
Copy link
Member

@colesbury, I'm going to merge this. Please keep an eye on the buildbots.

@ericsnowcurrently ericsnowcurrently merged commit 6e97a96 into python:main Oct 5, 2023
@colesbury colesbury deleted the thread_state branch October 5, 2023 16:39
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…ythongh-109915)

This adds a new field 'state' to PyThreadState that can take on one of three values: _Py_THREAD_ATTACHED, _Py_THREAD_DETACHED, or _Py_THREAD_GC.  The "attached" and "detached" states correspond closely to acquiring and releasing the GIL.  The "gc" state is current unused, but will be used to implement stop-the-world GC for --disable-gil builds in the near future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants