Disable PyGILState_Check() when Py_NewInterpreter() is called and add more checks on the GIL

[vstinner]

BPO	26558
Nosy	@pitrou, @vstinner, @ericsnowcurrently, @miss-islington
PRs	bpo-26558: Fix Py_FatalError() with GIL released #10267 [3.7] bpo-26558: Fix Py_FatalError() with GIL released (GH-10267) #10269 [3.6] bpo-26558: Fix Py_FatalError() with GIL released (GH-10267) #10270
Files	gil_check.patch gil_check-2.patch pymem_gil_check.patch

^{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 2016-03-15.16:16:43.293>
created_at = <Date 2016-03-14.13:16:41.679>
labels = []
title = 'Disable PyGILState_Check() when Py_NewInterpreter() is called and add more checks on the GIL'
updated_at = <Date 2018-11-01.00:11:57.241>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2018-11-01.00:11:57.241>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2016-03-15.16:16:43.293>
closer = 'vstinner'
components = []
creation = <Date 2016-03-14.13:16:41.679>
creator = 'vstinner'
dependencies = []
files = ['42161', '42162', '42163']
hgrepos = []
issue_num = 26558
keywords = ['patch']
message_count = 13.0
messages = ['261750', '261758', '261760', '261762', '261763', '261764', '261767', '261780', '261817', '261818', '329023', '329025', '329028']
nosy_count = 5.0
nosy_names = ['pitrou', 'vstinner', 'python-dev', 'eric.snow', 'miss-islington']
pr_nums = ['10267', '10269', '10270']
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue26558'
versions = ['Python 3.6']

[vstinner]

assert(PyGILState_Check()) cannot be used in functions called by Py_NewInterpreter(): the assertion fails.

I propose to add a flag to disable PyGILState_Check() while Py_NewInterpreter() is run to be able to add assertions in these functions.

The assertion helps to detect complex bugs like race condition in multithreaded applications.

[vstinner]

gil_check.patch: add more checks on the GIL

• PyGILState_Check() now returns 1 (success) before the creation of the GIL and
  after the destruction of the GIL

• Add a flag to disable PyGILState_Check(). Disable PyGILState_Check() when
  Py_NewInterpreter() is called

• Add assert(PyGILState_Check()) to:

  • _Py_dup()
  • _Py_fstat()
  • _Py_read()
  • _Py_write()
  • PyObject_Malloc()
  • PyObject_Calloc()
  • PyObject_Realloc()
  • PyObject_Free()

[vstinner]

I created this issue while working on the issue bpo-26249 "Change PyMem_Malloc to use pymalloc allocator". I would like to check that application call PyObject_Malloc() and PyMem_Malloc() with the GIL held.

[vstinner]

Hum, in fact I prefer to implement GIL check in debug hooks of Python memory allocators. So it becomes to get these checks on Python compiled in debug mode.

Patch 2: Don't touch Python memory allocators.

Instead of I wrote a second patch, pymem_gil_check.patch (based on gil_check-2.patch).

--

Oh, I forgot to mention that the gil_check patch changes PyThreadState_DeleteCurrent(): I reordered instructions to be able call PyObject_Free() with the GIL check.

[python-dev]

New changeset c24630367767 by Victor Stinner in branch 'default':
Fix Py_FatalError() if called without the GIL
https://hg.python.org/cpython/rev/c24630367767

[python-dev]

New changeset 8a5a0e223559 by Victor Stinner in branch 'default':
Issue bpo-26558: Remove useless check in tracemalloc
https://hg.python.org/cpython/rev/8a5a0e223559

[vstinner]

See also issues:

• bpo-10915: Make the PyGILState API compatible with multiple interpreters
• bpo-15751: Support subinterpreters in the GIL state API

This issue contains is a compromise: it's a workaround until the root issue is solve. Since bpo-10915 is open since 2011, I'm not sure that it's possible to fix the issue, nor that it's worth :-) To me, subinterpreters still looks like an experimental feature.

[python-dev]

New changeset e590c632c9fa by Victor Stinner in branch 'default':
Add more checks on the GIL
https://hg.python.org/cpython/rev/e590c632c9fa

New changeset cde3d986da00 by Victor Stinner in branch 'default':
Check the GIL in PyObject_Malloc()
https://hg.python.org/cpython/rev/cde3d986da00

[vstinner]

Ok, it looks like buildbots are happy. I close the issue.

[python-dev]

New changeset b394fc71f92a by Victor Stinner in branch '3.5':
Fix Py_FatalError() if called without the GIL
https://hg.python.org/cpython/rev/b394fc71f92a

New changeset c298c6d8b324 by Victor Stinner in branch '3.5':
faulthandler: Test Py_FatalError() with GIL released
https://hg.python.org/cpython/rev/c298c6d8b324

[vstinner]

New changeset 3a228ab by Victor Stinner in branch 'master':
bpo-26558: Fix Py_FatalError() with GIL released (GH-10267)
3a228ab

[miss-islington]

New changeset 192c547 by Miss Islington (bot) in branch '3.7':
bpo-26558: Fix Py_FatalError() with GIL released (GH-10267)
192c547

[vstinner]

New changeset ff56597 by Victor Stinner in branch '3.6':
bpo-26558: Fix Py_FatalError() with GIL released (GH-10267) (GH-10270)
ff56597