Skip to content

gh-114271: Clean Up _threadmodule.c #116776

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.

Sign up for GitHub

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

Closed
ericsnowcurrently wants to merge 14 commits into from
Closed

gh-114271: Clean Up _threadmodule.c #116776

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
db6bfcf
Make code sections more vistually distinct.
ericsnowcurrently Mar 13, 2024
db48323
Combine thread execution code into its own section.
ericsnowcurrently Mar 13, 2024
79c1d80
Make module function names more distinct.
ericsnowcurrently Mar 13, 2024
75d97c1
Group the module functions into subsections.
ericsnowcurrently Mar 13, 2024
3cace9e
Factor out thread_bootstate_new().
ericsnowcurrently Mar 13, 2024
0e7a0f7
Pass the boot state to do_start_new_thread().
ericsnowcurrently Mar 13, 2024
cec325e
Drop do_start_new_thread().
ericsnowcurrently Mar 13, 2024
4c83cf5
Move the thread execution code up near the top.
ericsnowcurrently Mar 13, 2024
23ca97b
Clean up the _PyThreadHandle code.
ericsnowcurrently Mar 13, 2024
41c7bd8
Factor out functions for tracking thread handles for fork.
ericsnowcurrently Mar 14, 2024
3db60f2
Factor out threadhandleobject.c.
ericsnowcurrently Mar 13, 2024
d2fb9bd
Make _ThreadHandle a static builtin type.
ericsnowcurrently Mar 13, 2024
4d0edee
Factor out struct _PyThread_handle_data.
ericsnowcurrently Mar 14, 2024
f1e8fa8
Track the data, not the objects.
ericsnowcurrently Mar 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Factor out thread_bootstate_new().
  • Loading branch information
@ericsnowcurrently
ericsnowcurrently committed Mar 13, 2024
commit 3cace9ea391c9e67b7cacec7446c92403f3d8f24
146 changes: 90 additions & 56 deletions Modules/_threadmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1302,21 +1302,98 @@ struct bootstate {
_PyEventRc *thread_is_exiting;
};

static struct bootstate *
thread_bootstate_new(PyObject *func, PyObject *args, PyObject *kwargs,
_PyEventRc *thread_is_exiting)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_THREADS)) {
PyErr_SetString(PyExc_RuntimeError,
"thread is not supported for isolated subinterpreters");
return NULL;
}
if (interp->finalizing) {
PyErr_SetString(PyExc_PythonFinalizationError,
"can't create new thread at interpreter shutdown");
return NULL;
}

PyThreadState *tstate = _PyThreadState_New(
interp, _PyThreadState_WHENCE_THREADING);
if (tstate == NULL) {
if (!PyErr_Occurred()) {
PyErr_NoMemory();
}
return NULL;
}

if (args == NULL) {
args = PyTuple_New(0);
if (args == NULL) {
PyThreadState_Clear(tstate);
PyThreadState_Delete(tstate);
}
}
else {
Py_INCREF(args);
}

// gh-109795: Use PyMem_RawMalloc() instead of PyMem_Malloc(),
// because it should be possible to call thread_bootstate_free()
// without holding the GIL.
struct bootstate *boot = PyMem_RawMalloc(sizeof(struct bootstate));
if (boot == NULL) {
PyErr_NoMemory();
Py_DECREF(args);
PyThreadState_Clear(tstate);
PyThreadState_Delete(tstate);
return NULL;
}
*boot = (struct bootstate){
.tstate = tstate,
.func = Py_NewRef(func),
.args = args,
.kwargs = Py_XNewRef(kwargs),
.thread_is_exiting = thread_is_exiting,
};
if (thread_is_exiting != NULL) {
_PyEventRc_Incref(thread_is_exiting);
}
return boot;
}

static void
thread_bootstate_free(struct bootstate *boot, int decref)
thread_bootstate_free(struct bootstate *boot)
{
if (decref) {
Py_DECREF(boot->func);
Py_DECREF(boot->args);
Py_XDECREF(boot->kwargs);
}
Py_XDECREF(boot->func);
Py_XDECREF(boot->args);
Py_XDECREF(boot->kwargs);
if (boot->thread_is_exiting != NULL) {
_PyEventRc_Decref(boot->thread_is_exiting);
}
if (boot->tstate != NULL) {
PyThreadState_Clear(boot->tstate);
// XXX PyThreadState_Delete() too?
}
PyMem_RawFree(boot);
}

static void
thread_bootstate_finalizing(struct bootstate *boot)
{
// Don't call PyThreadState_Clear() nor _PyThreadState_DeleteCurrent().
// These functions are called on tstate indirectly by Py_Finalize()
// which calls _PyInterpreterState_Clear().
//
// Py_DECREF() cannot be called because the GIL is not held: leak
// references on purpose. Python is being finalized anyway.
boot->tstate = NULL;
boot->func = NULL;
boot->args = NULL;
boot->kwargs = NULL;
thread_bootstate_free(boot);
}


static void
thread_run(void *boot_raw)
Expand All @@ -1337,13 +1414,7 @@ thread_run(void *boot_raw)
// At this stage, tstate can be a dangling pointer (point to freed memory),
// it's ok to call _PyThreadState_MustExit() with a dangling pointer.
if (_PyThreadState_MustExit(tstate)) {
// Don't call PyThreadState_Clear() nor _PyThreadState_DeleteCurrent().
// These functions are called on tstate indirectly by Py_Finalize()
// which calls _PyInterpreterState_Clear().
//
// Py_DECREF() cannot be called because the GIL is not held: leak
// references on purpose. Python is being finalized anyway.
thread_bootstate_free(boot, 0);
thread_bootstate_finalizing(boot);
goto exit;
}

Expand All @@ -1365,7 +1436,8 @@ thread_run(void *boot_raw)
Py_DECREF(res);
}

thread_bootstate_free(boot, 1);
boot->tstate = NULL;
thread_bootstate_free(boot);

_Py_atomic_add_ssize(&tstate->interp->threads.count, -1);
PyThreadState_Clear(tstate);
Expand All @@ -1390,41 +1462,11 @@ do_start_new_thread(thread_module_state* state,
PyThread_ident_t* ident, PyThread_handle_t* handle,
_PyEventRc *thread_is_exiting)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_THREADS)) {
PyErr_SetString(PyExc_RuntimeError,
"thread is not supported for isolated subinterpreters");
return -1;
}
if (interp->finalizing) {
PyErr_SetString(PyExc_PythonFinalizationError,
"can't create new thread at interpreter shutdown");
return -1;
}

// gh-109795: Use PyMem_RawMalloc() instead of PyMem_Malloc(),
// because it should be possible to call thread_bootstate_free()
// without holding the GIL.
struct bootstate *boot = PyMem_RawMalloc(sizeof(struct bootstate));
struct bootstate *boot = thread_bootstate_new(
func, args, kwargs, thread_is_exiting);
if (boot == NULL) {
PyErr_NoMemory();
return -1;
}
boot->tstate = _PyThreadState_New(interp, _PyThreadState_WHENCE_THREADING);
if (boot->tstate == NULL) {
PyMem_RawFree(boot);
if (!PyErr_Occurred()) {
PyErr_NoMemory();
}
return -1;
}
boot->func = Py_NewRef(func);
boot->args = Py_NewRef(args);
boot->kwargs = Py_XNewRef(kwargs);
boot->thread_is_exiting = thread_is_exiting;
if (thread_is_exiting != NULL) {
_PyEventRc_Incref(thread_is_exiting);
}

int err;
if (joinable) {
Expand All @@ -1436,8 +1478,7 @@ do_start_new_thread(thread_module_state* state,
}
if (err) {
PyErr_SetString(ThreadError, "can't start new thread");
PyThreadState_Clear(boot->tstate);
thread_bootstate_free(boot, 1);
thread_bootstate_free(boot);
return -1;
}
return 0;
Expand Down Expand Up @@ -1761,23 +1802,16 @@ threadmod_start_joinable_thread(PyObject *module, PyObject *func)
return NULL;
}

PyObject* args = PyTuple_New(0);
if (args == NULL) {
return NULL;
}
ThreadHandleObject* hobj = new_thread_handle(state);
if (hobj == NULL) {
Py_DECREF(args);
return NULL;
}
if (do_start_new_thread(state, func, args, /*kwargs=*/ NULL, /*joinable=*/ 1,
if (do_start_new_thread(state, func, /* args */ NULL, /*kwargs=*/ NULL, /*joinable=*/ 1,
&hobj->ident, &hobj->handle, hobj->thread_is_exiting)) {
Py_DECREF(args);
Py_DECREF(hobj);
return NULL;
}
set_thread_handle_state(hobj, THREAD_HANDLE_RUNNING);
Py_DECREF(args);
return (PyObject*) hobj;
}

Expand Down