PyThreadState_Next not thread safe

[jpe]

BPO	1021318
Nosy	@pitrou, @vstinner, @devdanzin, @jd, @ericsnowcurrently, @serhiy-storchaka

^{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:

assignee = None
closed_at = None
created_at = <Date 2004-09-02.16:56:47.000>
labels = ['interpreter-core', '3.9']
title = 'PyThreadState_Next not thread safe'
updated_at = <Date 2020-05-15.01:09:40.629>
user = 'https://bugs.python.org/jpe'

bugs.python.org fields:

activity = <Date 2020-05-15.01:09:40.629>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2004-09-02.16:56:47.000>
creator = 'jpe'
dependencies = []
files = []
hgrepos = []
issue_num = 1021318
keywords = []
message_count = 14.0
messages = ['60562', '82072', '114376', '358187', '358188', '358189', '358190', '358191', '358192', '358261', '358262', '358347', '364094', '368895']
nosy_count = 7.0
nosy_names = ['jpe', 'pitrou', 'vstinner', 'ajaksu2', 'jd', 'eric.snow', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue1021318'
versions = ['Python 3.9']

[jpe]

I can't convince myself that PyThreadState_Next is
thread safe even if the GIL is held because
PyGILState_Release can delete a thread state after
releasing the GIL if the gilstate_counter reaches 0.
So another thread could be deleting a thread state my
thread is referencing.

If the GIL was held when the thread state was deleted,
the I think the next function would be thread safe
because new thread states are always added to the front
of the list.

If these functions should work when the GIL is not
held, the head_mutex in pystate.c probably needs to be
exposed in some manner so code can lock it, iterate
through the threads, and then unlock it.

[devdanzin]

Can someone take a look a this and either close or propose what to do?

[BreamoreBoy]

No reply to msg82072.

[jd]

This is actually true and it's quite easy to produce a bug that triggers a segmentation fault when iterating over a large list of threads create dynamically.

That makes PyThreadState_Next barely usable for any program that has a few threads. The head mutex needs to be locked to be able to iterate safely over the list of thread.

[vstinner]

This is actually true and it's quite easy to produce a bug that triggers a segmentation fault when iterating over a large list of threads create dynamically.

This issue is closed. Would you be able to write a reproducer script? If yes, maybe open a new issue, or I can reopen this one.

[jd]

It'd be great if you could reopen it. I'm interested in fixing it properly.

It's impossible to "solve" in Python 2 since the head mutex is not accessible (it's static in pystate.c) but this is doable with Python 3 (yay).

We'd simply need to provide a new API to lock/unlock the interpreter's mutex.

How hard to you need the script to reproduce? I can try to spend some time to extract some of my code to have one if that's really needed (I'm using Cython to generate some of the C code).

[vstinner]

I reopened the issue and changed the version to 3.9.

[vstinner]

We'd simply need to provide a new API to lock/unlock the interpreter's mutex.

Are you talking about this lock?

#define HEAD_LOCK(runtime) \
    PyThread_acquire_lock((runtime)->interpreters.mutex, WAIT_LOCK)
#define HEAD_UNLOCK(runtime) \
    PyThread_release_lock((runtime)->interpreters.mutex)

[jd]

Yes, that's the one 👍

[jd]

Victor, do you have an idea of how you would like this mutex to be exposed?

I see it only recently changed in Python 3.7 to be part of a structure, though this structure is not public AFAIU.

Is adding 2 new lock/unlock C functions good enough?

[vstinner]

I'm not sure of what is the best approach. Currently, bpo-10915 is blocking different projects: "Make the PyGILState API compatible with multiple interpreters". We may have to fix this API first, and clarify the scope of the different locks.

[ericsnowcurrently]

On Wed, Dec 11, 2019 at 7:02 AM STINNER Victor <report@bugs.python.org> wrote:

We may have to fix this API first, and clarify the scope of the different locks.

+1

[vstinner]

See also bpo-35370: "Add _PyEval_SetTrace(tstate, func, arg) function".

[vstinner]

I marked bpo-26461 as a duplicate of this issue.