homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: co_extra_freefuncs is stored thread locally and can lead to crashes
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: dino.viehland Nosy List: Mariatta, brett.cannon, dino.viehland, jeethu, ned.deily, serhiy.storchaka, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2017年06月08日 21:37 by dino.viehland, last changed 2022年04月11日 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 2015 merged dino.viehland, 2017年06月08日 21:43
PR 2144 merged dino.viehland, 2017年06月13日 04:48
PR 2152 merged vstinner, 2017年06月13日 08:21
PR 2356 merged vstinner, 2017年06月23日 12:57
PR 2358 merged vstinner, 2017年06月23日 13:10
PR 2455 merged vstinner, 2017年06月28日 00:00
PR 2456 merged vstinner, 2017年06月28日 00:17
Messages (21)
msg295467 - (view) Author: Dino Viehland (dino.viehland) * (Python committer) Date: 2017年06月08日 21:37
The co_extra_freefuncs are stored in PyThreadState. When calling _PyEval_RequestCodeExtraIndex you are given a thread specific index. The code object can then lose it's last reference on a different thread, and the wrong free function can be called if users of the extra space have made calls to get their index in different orders. 
This can also lead to crashes if the extra thread hasn't yet requested extra indexes either.
msg295477 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017年06月09日 01:23
See review comments on PR 2015.
msg295489 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017年06月09日 05:40
User code shouldn't allocate PyInterpreterState and PyThreadState structures, it only uses structures created by the interpreter. Changing the size of PyInterpreterState should be safe. The only possible breaking compatibility is if user code directly access co_extra_user_count, co_extra_freefuncs, async_gen_firstiter or async_gen_finalizer rather than using the API: _PyEval_RequestCodeExtraIndex(), _PyCode_GetExtra(), _PyCode_SetExtra(), _PyEval_GetAsyncGenFirstiter(), _PyEval_SetAsyncGenFirstiter(), _PyEval_GetAsyncGenFinalizer() and _PyEval_SetAsyncGenFinalizer().
Nick's idea about _preserve_36_ABI_1 and _preserve_36_ABI_2 should address concerns about direct access to async_gen_firstiter and async_gen_finalizer. Direct access to co_extra_user_count and co_extra_freefuncs obviously can't be preserved.
PyInterpreterState and PyThreadState are not in the stable ABI. They are opaque types when use limited API. May be they should be made opaque for all user code (in 3.7+).
msg295835 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017年06月13日 01:46
New changeset 2997fec01ee7300c6d5940e6c55e4ccf9f56f1b5 by Ned Deily (Dino Viehland) in branch '3.6':
[3.6] bpo-30604: Move co_extra_freefuncs to interpreter state to avoid crashes in threads (#2015)
https://github.com/python/cpython/commit/2997fec01ee7300c6d5940e6c55e4ccf9f56f1b5
msg295845 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017年06月13日 04:58
Avoid using double underscores in C code. C compiler uses names with double underscores for its own needs, and this can lead to conflicts.
msg295868 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年06月13日 08:39
New changeset 932946ca14168e556293d2508c8eebb23a56a2b2 by Victor Stinner in branch '3.6':
bpo-30604: Fix __PyCodeExtraState_Get() prototype (#2152)
https://github.com/python/cpython/commit/932946ca14168e556293d2508c8eebb23a56a2b2
msg296301 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017年06月19日 03:20
I'm setting the stage to 'backport needed', but it really is a 'porting needed' stage :)
The two PRs merged PRs here were made against 3.6.
msg296588 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017年06月21日 21:44
New changeset f3cffd2b7879d209f982de899b782fb89cfc410a by Yury Selivanov (Dino Viehland) in branch 'master':
bpo-30604: clean up co_extra support (#2144)
https://github.com/python/cpython/commit/f3cffd2b7879d209f982de899b782fb89cfc410a
msg296595 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2017年06月21日 22:57
Should this be closed since the all PRs got merged?
msg296596 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017年06月21日 23:07
PR 2152 is not yet ported to master.
msg296597 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017年06月21日 23:15
It doesn't need to be, it's only for 3.6
msg296599 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017年06月21日 23:20
Closing the issue. Thank you Dino for working on this!
msg296607 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年06月22日 00:03
If a test requires ctypes, please skip your test if ctypes is missing. The new test fails on the "x86 Ubuntu Shared 3.x" buildbot which lacks the _ctypes module (for an unknown reason, but does it really matter here? ;-)).
http://buildbot.python.org/all/builders/x86%20Ubuntu%20Shared%203.x/builds/924/steps/test/logs/stdio
test test_code crashed -- Traceback (most recent call last):
 File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/libregrtest/runtest.py", line 156, in runtest_inner
 the_module = importlib.import_module(abstest)
 File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/importlib/__init__.py", line 127, in import_module
 return _bootstrap._gcd_import(name[level:], package, level)
 File "<frozen importlib._bootstrap>", line 978, in _gcd_import
 File "<frozen importlib._bootstrap>", line 961, in _find_and_load
 File "<frozen importlib._bootstrap>", line 950, in _find_and_load_unlocked
 File "<frozen importlib._bootstrap>", line 655, in _load_unlocked
 File "<frozen importlib._bootstrap_external>", line 679, in exec_module
 File "<frozen importlib._bootstrap>", line 205, in _call_with_frames_removed
 File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_code.py", line 218, in <module>
 import ctypes
 File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/ctypes/__init__.py", line 7, in <module>
 from _ctypes import Union, Structure, Array
ModuleNotFoundError: No module named '_ctypes'
msg296709 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年06月23日 13:08
New changeset a4b091e135ccf345cfafdd8477aef897c5214f82 by Victor Stinner in branch 'master':
bpo-30604: Skip CoExtra tests if ctypes is missing (#2356)
https://github.com/python/cpython/commit/a4b091e135ccf345cfafdd8477aef897c5214f82
msg296718 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年06月23日 13:24
New changeset cea2174ab7cce01c420b2770562be4c91f1f4e35 by Victor Stinner in branch '3.6':
bpo-30604: Skip CoExtra tests if ctypes is missing (#2356) (#2358)
https://github.com/python/cpython/commit/cea2174ab7cce01c420b2770562be4c91f1f4e35
msg296719 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年06月23日 13:26
The test_code is fixed again, so I close the issue.
msg297076 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年06月28日 00:03
_PyCode_SetExtra() uses two memory block for code extras. By changing how memory is accessed and allocated, it would be possible to use a single memory block. Was it on purpose to use two memory blocks?
See for example PyTupleObject which uses a single memory block vs PyListObject which uses two memory blocks.
typedef struct {
 PyObject_VAR_HEAD
 PyObject *ob_item[1];
 /* ob_item contains space for 'ob_size' elements.
 * Items must normally not be NULL, except during construction when
 * the tuple is not yet visible outside the function that builds it.
 */
} PyTupleObject;
msg297078 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年06月28日 00:11
> _PyCode_SetExtra() uses two memory block for code extras. By changing how memory is accessed and allocated, it would be possible to use a single memory block. Was it on purpose to use two memory blocks?
I discussed with Yury who is not opposed to such change in Python 3.7, so I created bpo-30789.
msg297079 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年06月28日 00:12
New changeset 23e7944eba1968bb8432fdc4cc96d4fdd2c1a230 by Victor Stinner in branch 'master':
bpo-30704, bpo-30604: Fix memleak in code_dealloc() (#2455)
https://github.com/python/cpython/commit/23e7944eba1968bb8432fdc4cc96d4fdd2c1a230
msg297082 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年06月28日 00:28
New changeset 26daad4ee14693381d84a5235709d22aed1c22ed by Victor Stinner in branch '3.6':
bpo-30704, bpo-30604: Fix memleak in code_dealloc() (#2455) (#2456)
https://github.com/python/cpython/commit/26daad4ee14693381d84a5235709d22aed1c22ed
msg297938 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017年07月08日 04:51
New changeset c794b643c9172d69afa46f85982befd82511d9df by Ned Deily (Victor Stinner) in branch '3.6':
bpo-30704, bpo-30604: Fix memleak in code_dealloc() (#2455) (#2456)
https://github.com/python/cpython/commit/c794b643c9172d69afa46f85982befd82511d9df
History
Date User Action Args
2022年04月11日 14:58:47adminsetgithub: 74789
2018年01月17日 02:43:32jeethusetnosy: + jeethu
2017年07月08日 04:51:41ned.deilysetmessages: + msg297938
2017年06月28日 00:28:53vstinnersetmessages: + msg297082
2017年06月28日 00:17:10vstinnersetpull_requests: + pull_request2514
2017年06月28日 00:12:03vstinnersetmessages: + msg297079
2017年06月28日 00:11:46vstinnersetmessages: + msg297078
2017年06月28日 00:03:54vstinnersetmessages: + msg297076
2017年06月28日 00:00:45vstinnersetpull_requests: + pull_request2512
2017年06月23日 13:26:09vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg296719
2017年06月23日 13:24:30vstinnersetmessages: + msg296718
2017年06月23日 13:10:58vstinnersetpull_requests: + pull_request2405
2017年06月23日 13:08:57vstinnersetmessages: + msg296709
2017年06月23日 12:57:16vstinnersetpull_requests: + pull_request2403
2017年06月22日 00:03:45vstinnersetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg296607
2017年06月21日 23:20:31yselivanovsetstatus: open -> closed
resolution: fixed
messages: + msg296599

stage: backport needed -> resolved
2017年06月21日 23:15:42yselivanovsetmessages: + msg296597
2017年06月21日 23:07:40Mariattasetmessages: + msg296596
2017年06月21日 22:57:35brett.cannonsetmessages: + msg296595
2017年06月21日 21:44:38yselivanovsetmessages: + msg296588
2017年06月19日 03:20:05Mariattasetnosy: + Mariatta

messages: + msg296301
stage: patch review -> backport needed
2017年06月13日 08:39:33vstinnersetnosy: + vstinner
messages: + msg295868
2017年06月13日 08:21:04vstinnersetpull_requests: + pull_request2204
2017年06月13日 04:58:01serhiy.storchakasetmessages: + msg295845
2017年06月13日 04:48:50dino.viehlandsetpull_requests: + pull_request2194
2017年06月13日 01:46:37ned.deilysetmessages: + msg295835
2017年06月09日 05:40:31serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg295489
2017年06月09日 01:23:39ned.deilysetnosy: + ned.deily

messages: + msg295477
versions: + Python 3.7
2017年06月08日 21:43:03dino.viehlandsetpull_requests: + pull_request2080
2017年06月08日 21:37:03dino.viehlandcreate

AltStyle によって変換されたページ (->オリジナル) /