Race condition using PyGILState_Ensure on a new thread

[syeberman]

BPO	12179
Nosy	@pitrou, @vstinner

^{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 = <Date 2019-10-23.00:26:38.256>
created_at = <Date 2011-05-25.19:58:14.182>
labels = ['interpreter-core']
title = 'Race condition using PyGILState_Ensure on a new thread'
updated_at = <Date 2019-10-23.00:26:38.254>
user = 'https://bugs.python.org/syeberman'

bugs.python.org fields:

activity = <Date 2019-10-23.00:26:38.254>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-10-23.00:26:38.256>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2011-05-25.19:58:14.182>
creator = 'syeberman'
dependencies = []
files = []
hgrepos = []
issue_num = 12179
keywords = []
message_count = 3.0
messages = ['136888', '221433', '355197']
nosy_count = 4.0
nosy_names = ['syeberman', 'pitrou', 'vstinner', 'neologix']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue12179'
versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2', 'Python 3.3']

[syeberman]

I'm wanting to call PyThreadState_SetAsyncExc from a function registered with SetConsoleCtrlHandler. To do so, I need to call PyGILState_Ensure, which asserts that Python is initialized, so I need to check for that. However, I noticed a race condition with the code:

  if( Py_IsInitialized( ) ) {
    // XXX What if another thread calls Py_Finalize here?
    gstate = PyGILState_Ensure( );
    PyThreadState_SetAsyncExc( MainThreadId, PyExc_SystemExit );
    PyGILState_Release( gstate );
  }

What I need is to be able to hold the GIL around the entire block of code, potentially before Py_Initialize is called for the first time. Now, 3.2 deprecated PyEval_AcquireLock, and PyEval_InitThreads can no longer be called before Py_Initialize. Thankfully, I'm on 2.6.4, so I was able to write this code:

  PyEval_AcquireLock( );
  if( Py_IsInitialized( ) ) {
    gstate = PyGILState_Ensure( );
    PyThreadState_SetAsyncExc( MainThreadId, PyExc_SystemExit );
    PyGILState_Release( gstate );
  }
  PyEval_ReleaseLock( );

The problem in 2.6.4 is that PyGILState_Ensure deadlocks because the GIL is already held, so that doesn't solve my problem. (Incidentally, the PyGILState_Ensure docs say it works "regardless of the current state of the GIL", which is incorrect.)

The 3.2 docs say to use PyEval_AcquireThread or PyEval_RestoreThread, which both require an existing PyThreadState. To get that, I would need to call PyThreadState_New, which needs a PyInterpreterState. To get _that_ I could use PyInterpreterState_Head, since I know I only use one interpreter. Now I'm getting into "advanced debugger" territory, but it's no use anyway; it's possible that Py_Finalize could sneak in between the time I get this interpreter and when I acquire the GIL, causing me to access a free'd interpreter.

I believe the best fix for this would be to have a version of PyGILState_Ensure that works even when Python is not initialized. It would not be able to create a thread, and thus I would not expect to be able to call any Python API, but it would always ensure the GIL is acquired. This _may_ have to be a new "PyGILState_EnsureEx" function, because existing code expects PyGILState_Ensure to always allow them to call the Python API. The resulting code would be:

  gstate = PyGILState_EnsureEx( );
  if( Py_IsInitialized( ) ) {
    PyThreadState_SetAsyncExc( MainThreadId, PyExc_SystemExit );
  }
  PyGILState_Release( gstate );

This would require that Py_Initialize itself acquires the GIL.

The above problem was found on 2.6.4, but I've consulted the 3.2 docs and 3.3 code (via the online source) and it looks like the situation would be exactly the same. In the meantime, I'm going to stick with the first piece of code and hope nobody hits CTRL-BREAK during program clean-up.

[BreamoreBoy]

Can one of you on the nosy list pick this up please, it's way out of my league.

[vstinner]

Bug report in 2011. I close this old issue.