msg349881 - (view) |
Author: Joannah Nanjekye (nanjekyejoannah) * |
Date: 2019-08-16 19:53 |
I just noticed that PyThreadState_DeleteCurrent()in Python/pystate.c is not documented.
Relevant documentation should go in Doc/c-api/init.rst If no one objects to this.
|
msg349954 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-08-19 10:40 |
I'm not sure why PyThreadState_DeleteCurrent() is exposed. Why would anyone use it?
It is used internally by the _thread module when the thread function completes.
I suggest to make the function internal instead of documenting it.
Eric, Joannah: what do you think?
|
msg349960 - (view) |
Author: Joannah Nanjekye (nanjekyejoannah) * |
Date: 2019-08-19 15:31 |
From what I know the GIL is released by calling PyEval_ReleaseThread() or PyEval_SaveThread().
I see we can use PyThreadState_Delete() to clean up-specifically destroy thread state but does not release the GIL and requires a thread state. On the other hand, PyThreadState_DeleteCurrent() allows to both clean up the current thread state - no thread state is required as well release the GIL which is convenient. I would not fight so much to keep it public given we have PyEval_ReleaseThread() or PyEval_SaveThread().
|
msg349979 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-08-19 23:33 |
Is it safe to call PyThreadState_DeleteCurrent()?
|
msg350671 - (view) |
Author: Joannah Nanjekye (nanjekyejoannah) * |
Date: 2019-08-28 16:32 |
I updated the PR to make the function internal.
|
msg350773 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-08-29 13:09 |
I did a GitHub code search on "PyThreadState_DeleteCurrent" in C code:
https://github.com/search?l=C&q=PyThreadState_DeleteCurrent&type=Code
I looked at the first 6 pages: I only found copies of the Python source code, but no *call* to this function. It seems like this function is not used outside Python, so it's fine to remove it.
Anyway, if an user complains lately, we can add it back. It's fine.
In case of doubt, in this case, I'm fine with removing the function.
|
msg351210 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-09-05 16:06 |
New changeset 2bc43cdc015eda4f1a651bb2b308a17a83c38e14 by Victor Stinner (Joannah Nanjekye) in branch 'master':
bpo-37878: Remove PyThreadState_DeleteCurrent() function (GH-15315)
https://github.com/python/cpython/commit/2bc43cdc015eda4f1a651bb2b308a17a83c38e14
|
msg351211 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-09-05 16:07 |
Thanks Joannah.
|
msg353079 - (view) |
Author: Joannah Nanjekye (nanjekyejoannah) * |
Date: 2019-09-24 12:06 |
Update, Some user of this function just complained on the merged pull requests here : https://github.com/python/cpython/pull/15315#issuecomment-534522962.
Do we revert and deprecate ? cc @victor
|
msg353080 - (view) |
Author: Wenzel Jakob (wenzel) |
Date: 2019-09-24 12:19 |
Hi,
pybind11 developer here. A bit on context of our usage of this function:
Pybind11 supports some advanced GIL-related tricks that *require* access this function. For instance, pybind11 can intercept a Python function call on the `main()` thread and delete the associated thread state, launching a new thread and continuing Python execution there (with a newly created thread state).
Kind of crazy, but why is this useful? UI libraries. On some platforms, it is not legal to poll UI events on any thread other than the main thread. This means that it's impossible to implement a proper UI event loop because Python is hogging the main thread. With the functionality in pybind11's ``gil_scoped_acquire``, I can launch an event polling loop on the main thread, continue running an interactive Python session on another thread, and even swap them back e.g. when the user interface is no longer needed.
Best,
Wenzel
|
msg353369 - (view) |
Author: Eric Snow (eric.snow) * |
Date: 2019-09-27 14:17 |
I've re-opened this issue to address the original point: documenting PyThreadState_DeleteCurrent(). FWIW, I don't see a problem with documenting it if we're keeping it around (which it looks like we are).
We will address reverting GH-15315 in #38266.
|
msg353923 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-10-04 11:35 |
New changeset 8855e47d09d4db1206c65b24efc8ad0df585ac46 by Victor Stinner (Joannah Nanjekye) in branch 'master':
bpo-38266: Revert bpo-37878: Make PyThreadState_DeleteCurrent() Internal (GH-16558)
https://github.com/python/cpython/commit/8855e47d09d4db1206c65b24efc8ad0df585ac46
|
msg353925 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-10-04 11:45 |
> bpo-38266: Revert bpo-37878: Make PyThreadState_DeleteCurrent() Internal (GH-16558)
Oh, I didn't notice something in the review: the revert moved PyThreadState_DeleteCurrent() definition from Include/pystate.h to Include/cpython/pystate.h. Previously, it was defined in Include/pystate.h, see the commit removing it:
https://github.com/python/cpython/commit/2bc43cdc015eda4f1a651bb2b308a17a83c38e14
Joannah: can you please move the definition back to Include/pystate.h, at the same place it was before the removal, please?
The revert excluded PyThreadState_DeleteCurrent() from the stable API (Py_LIMITED_API).
|
msg353927 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-10-04 11:46 |
To be clear: this issue only impacts Python 3.8. The function was only removed from the master branch. It was still in the 3.8 branch.
|
msg353936 - (view) |
Author: Joannah Nanjekye (nanjekyejoannah) * |
Date: 2019-10-04 12:13 |
> Oh, I didn't notice something in the review: the revert moved >PyThreadState_DeleteCurrent() definition from Include/pystate.h to >Include/cpython/pystate.h. Previously, it was defined in > Include/pystate.h, see the commit removing it:
I made this change as it was requested by Eric snow in issue38266.
"when we un-revert we should be sure to move PyThreadState_DeleteCurrent() from Include/pystate.h to Include/cpython/pystate.h."
|
msg353949 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-10-04 16:01 |
> "when we un-revert we should be sure to move PyThreadState_DeleteCurrent() from Include/pystate.h to Include/cpython/pystate.h."
Ah, I didn't see this comment. If Eric endorses this change, I'm fine with it :-)
I close the issue.
If someone wants to modify PyThreadState_DeleteCurrent() to automatically call PyThreadState_Clear(), please open a new issue for that.
|
msg370236 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2020-05-28 16:26 |
New changeset fda7f6d61b13c68f59806db674e892fda4013348 by Victor Stinner in branch 'master':
bpo-37878: PyThreadState_DeleteCurrent() was not removed (GH-20489)
https://github.com/python/cpython/commit/fda7f6d61b13c68f59806db674e892fda4013348
|
msg370237 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2020-05-28 16:27 |
I was very confused by the history of this issue. To be clear, PyThreadState_DeleteCurrent() is now documented in Python 3.9:
https://docs.python.org/dev/c-api/init.html#c.PyThreadState_DeleteCurrent
|
msg370240 - (view) |
Author: miss-islington (miss-islington) |
Date: 2020-05-28 16:34 |
New changeset 6a478641c00e5a52be9d595ad804bf848a498d96 by Miss Islington (bot) in branch '3.9':
bpo-37878: PyThreadState_DeleteCurrent() was not removed (GH-20489)
https://github.com/python/cpython/commit/6a478641c00e5a52be9d595ad804bf848a498d96
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:19 | admin | set | github: 82059 |
2020-05-28 16:34:50 | miss-islington | set | messages:
+ msg370240 |
2020-05-28 16:27:27 | vstinner | set | messages:
+ msg370237 |
2020-05-28 16:26:16 | miss-islington | set | nosy:
+ miss-islington
pull_requests:
+ pull_request19745 |
2020-05-28 16:26:09 | vstinner | set | messages:
+ msg370236 |
2020-05-28 14:34:21 | vstinner | set | pull_requests:
+ pull_request19738 |
2019-10-04 16:01:16 | vstinner | set | status: open -> closed resolution: fixed messages:
+ msg353949
stage: patch review -> resolved |
2019-10-04 12:13:03 | nanjekyejoannah | set | messages:
+ msg353936 |
2019-10-04 11:46:39 | vstinner | set | messages:
+ msg353927 |
2019-10-04 11:45:51 | vstinner | set | messages:
+ msg353925 versions:
- Python 3.8 |
2019-10-04 11:35:53 | vstinner | set | messages:
+ msg353923 |
2019-10-03 12:52:18 | nanjekyejoannah | set | stage: needs patch -> patch review pull_requests:
+ pull_request16149 |
2019-09-27 14:17:59 | eric.snow | set | status: closed -> open resolution: fixed -> (no value) messages:
+ msg353369
stage: resolved -> needs patch |
2019-09-24 12:19:09 | wenzel | set | nosy:
+ wenzel messages:
+ msg353080
|
2019-09-24 12:06:42 | nanjekyejoannah | set | messages:
+ msg353079 |
2019-09-05 16:07:34 | vstinner | set | status: open -> closed resolution: fixed messages:
+ msg351211
stage: patch review -> resolved |
2019-09-05 16:06:54 | vstinner | set | messages:
+ msg351210 |
2019-08-29 13:09:30 | vstinner | set | messages:
+ msg350773 |
2019-08-28 16:32:48 | nanjekyejoannah | set | messages:
+ msg350671 |
2019-08-19 23:33:17 | vstinner | set | keywords:
- easy
messages:
+ msg349979 |
2019-08-19 15:31:01 | nanjekyejoannah | set | messages:
+ msg349960 |
2019-08-19 10:40:00 | vstinner | set | messages:
+ msg349954 |
2019-08-16 20:00:06 | nanjekyejoannah | set | keywords:
+ patch stage: needs patch -> patch review pull_requests:
+ pull_request15034 |
2019-08-16 19:57:21 | nanjekyejoannah | set | title: Sub-ineterpreters : Document PyThreadState_DeleteCurrent() ? -> Sub-Interpreters : Document PyThreadState_DeleteCurrent() |
2019-08-16 19:55:46 | nanjekyejoannah | set | title: sub-ineterpreters : Document PyThreadState_DeleteCurrent() ? -> Sub-ineterpreters : Document PyThreadState_DeleteCurrent() ? |
2019-08-16 19:53:55 | nanjekyejoannah | create | |