classification
Title: Disable PyGILState_Check() when Py_NewInterpreter() is called and add more checks on the GIL
Type: Stage:
Components: Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eric.snow, miss-islington, pitrou, python-dev, vstinner
Priority: normal Keywords: patch

Created on 2016-03-14 13:16 by vstinner, last changed 2018-11-01 00:11 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
gil_check.patch vstinner, 2016-03-14 14:56 review
gil_check-2.patch vstinner, 2016-03-14 16:00 review
pymem_gil_check.patch vstinner, 2016-03-14 16:00
Pull Requests
URL Status Linked Edit
PR 10267 merged vstinner, 2018-10-31 21:39
PR 10269 merged miss-islington, 2018-10-31 23:27
PR 10270 merged vstinner, 2018-10-31 23:34
Messages (13)
msg261750 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-03-14 13:16
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.
msg261758 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-03-14 14:56
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()
msg261760 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-03-14 15:09
I created this issue while working on the issue #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.
msg261762 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-03-14 16:00
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.
msg261763 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-14 16:01
New changeset c24630367767 by Victor Stinner in branch 'default':
Fix Py_FatalError() if called without the GIL
https://hg.python.org/cpython/rev/c24630367767
msg261764 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-14 16:02
New changeset 8a5a0e223559 by Victor Stinner in branch 'default':
Issue #26558: Remove useless check in tracemalloc
https://hg.python.org/cpython/rev/8a5a0e223559
msg261767 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-03-14 16:16
See also issues:

* #10915: Make the PyGILState API compatible with multiple interpreters 
* #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 #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.
msg261780 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-14 21:50
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
msg261817 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-03-15 16:16
Ok, it looks like buildbots are happy. I close the issue.
msg261818 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-15 16:26
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
msg329023 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-10-31 23:26
New changeset 3a228ab17c2a9cffd1a2f15f30d6209768de20a6 by Victor Stinner in branch 'master':
bpo-26558: Fix Py_FatalError() with GIL released (GH-10267)
https://github.com/python/cpython/commit/3a228ab17c2a9cffd1a2f15f30d6209768de20a6
msg329025 - (view) Author: miss-islington (miss-islington) Date: 2018-10-31 23:45
New changeset 192c54713b4c67a8d14b2d98056771feba40ca37 by Miss Islington (bot) in branch '3.7':
bpo-26558: Fix Py_FatalError() with GIL released (GH-10267)
https://github.com/python/cpython/commit/192c54713b4c67a8d14b2d98056771feba40ca37
msg329028 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-01 00:11
New changeset ff56597151206a03ce421712516323430b7749c8 by Victor Stinner in branch '3.6':
bpo-26558: Fix Py_FatalError() with GIL released (GH-10267) (GH-10270)
https://github.com/python/cpython/commit/ff56597151206a03ce421712516323430b7749c8
History
Date User Action Args
2018-11-01 00:11:57vstinnersetmessages: + msg329028
2018-10-31 23:47:21eric.snowsetnosy: + eric.snow
2018-10-31 23:45:46miss-islingtonsetnosy: + miss-islington
messages: + msg329025
2018-10-31 23:34:08vstinnersetpull_requests: + pull_request9580
2018-10-31 23:27:03miss-islingtonsetpull_requests: + pull_request9579
2018-10-31 23:26:44vstinnersetmessages: + msg329023
2018-10-31 21:39:43vstinnersetpull_requests: + pull_request9578
2016-03-15 16:26:07python-devsetmessages: + msg261818
2016-03-15 16:16:43vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg261817
2016-03-14 21:50:03python-devsetmessages: + msg261780
2016-03-14 16:20:08vstinnersettitle: Fix PyGILState_Check() with _testcapi.run_in_subinterp() -> Disable PyGILState_Check() when Py_NewInterpreter() is called and add more checks on the GIL
2016-03-14 16:16:08vstinnersetnosy: + pitrou
messages: + msg261767
2016-03-14 16:02:09python-devsetmessages: + msg261764
2016-03-14 16:01:02python-devsetnosy: + python-dev
messages: + msg261763
2016-03-14 16:00:23vstinnersetfiles: + pymem_gil_check.patch
2016-03-14 16:00:11vstinnersetfiles: + gil_check-2.patch

messages: + msg261762
2016-03-14 15:09:34vstinnersetmessages: + msg261760
2016-03-14 14:56:27vstinnersetfiles: + gil_check.patch
keywords: + patch
messages: + msg261758
2016-03-14 13:16:41vstinnercreate