classification
Title: [subinterpreters] Global C variables are a problem
Type: behavior Stage: patch review
Components: Subinterpreters Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: eric.snow Nosy List: christian.heimes, eric.snow, maciej.szulik, pablogsal, phsilva, vinay.sajip, vstinner
Priority: normal Keywords: patch

Created on 2019-05-10 19:01 by eric.snow, last changed 2020-11-20 22:39 by eric.snow.

Pull Requests
URL Status Linked Edit
PR 13372 closed eric.snow, 2019-05-17 00:46
PR 13531 closed eric.snow, 2019-05-23 22:55
PR 15760 closed eric.snow, 2019-09-09 11:30
PR 15877 merged eric.snow, 2019-09-10 15:31
PR 16017 merged eric.snow, 2019-09-12 09:23
PR 16058 merged eric.snow, 2019-09-12 14:41
PR 16328 merged vinay.sajip, 2019-09-22 04:41
PR 16841 merged eric.snow, 2019-10-18 21:07
PR 22841 merged eric.snow, 2020-10-21 00:18
PR 23045 merged eric.snow, 2020-10-30 20:53
PR 23431 merged eric.snow, 2020-11-20 21:52
Messages (16)
msg342119 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-05-10 19:01
We still have a bunch of "global" C variables (static globals, static locals, maybe thread-local storage) in our code-base that break the isolation between interpreters.  I added Tools/c-globals/check-c-globals.py a while back to help identify such variables, however more have crept in.  I also did not take static locals or thread-locals into account.

To address the above, we should do the following:

1. update check-c-globals.py to identify static locals (and thread-locals)
2. deal with any identified globals
   * move them to _PyRuntimeState (or thread-locals to PyThreadState, etc.)
   * ignore them by adding them to Tools/c-globals/ignored-globals.txt
3. add check-c-globals.py to "make check"
4. (if "make check" isn't already there), ensure check-c-globals.py is run at some point in CI

Separately, we should move fields out of _PyRuntimeState into PyInterpreterState wherever possible.  That can also be done at step 2 if it's not too much work.
msg342125 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-05-10 20:13
Also, Tools/c-globals/ignored-globals.txt is a bit out of date (some vars have been removed, renamed, or moved to another file).  That should get cleaned up.  It might also make sense to update check-c-globals.py to verify that all variables in ignored-globals.txt actually exist.
msg352013 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-09-11 18:49
New changeset ee536b2020b1f0baad1286dbd4345e13870324af by Eric Snow in branch 'master':
bpo-36876: Add a tool that identifies unsupported global C variables. (#15877)
https://github.com/python/cpython/commit/ee536b2020b1f0baad1286dbd4345e13870324af
msg352032 - (view) Author: David Bolen (db3l) Date: 2019-09-11 23:37
The new test_check_c_globals.ActualChecks test is failing with an "unexpected success" on the bolen-ubuntu buildbot (under Ubuntu 18.04.3).  I can reproduce the failure in a manually built tree.
msg352063 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-09-12 09:03
@db3l, I'll take a look right away.
msg352085 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-09-12 09:51
New changeset 64535fc6c0712caef0bc46be30e661f7ccf8280e by Eric Snow in branch 'master':
bpo-36876: Skip test_check_c_globals for now. (gh-16017)
https://github.com/python/cpython/commit/64535fc6c0712caef0bc46be30e661f7ccf8280e
msg352208 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-09-12 15:59
New changeset 088b63ea7a8331a3e34bc93c3b873c60354b4fad by Eric Snow in branch 'master':
bpo-36876: Fix the globals checker tool. (gh-16058)
https://github.com/python/cpython/commit/088b63ea7a8331a3e34bc93c3b873c60354b4fad
msg354928 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-10-19 02:00
New changeset e4c431ecf50def40eb93c3969c1e4eeaf7bf32f1 by Eric Snow in branch 'master':
bpo-36876: Re-organize the c-analyzer tool code. (gh-16841)
https://github.com/python/cpython/commit/e4c431ecf50def40eb93c3969c1e4eeaf7bf32f1
msg356181 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-11-07 10:09
New changeset 9def81aa52adc3cc89554156e40742cf17312825 by Vinay Sajip in branch 'master':
bpo-36876: Moved Parser/listnode.c statics to interpreter state. (GH-16328)
https://github.com/python/cpython/commit/9def81aa52adc3cc89554156e40742cf17312825
msg357351 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-11-23 00:41
Thanks, Vinay!
msg357352 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-11-23 00:42
FYI, others have been tackling this in separate issues (e.g. Victor, anyone relative to PEP 384).
msg357353 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-11-23 00:43
And I *am* still working on the c-analyzer + a test to that runs it, so we can keep from adding more globals. :)
msg368910 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-05-15 01:37
More and more C extensions are converted to multiphase initialization API (PEP 489) and their global variables are moved into a new module state.

bpo-39465 will make _Py_IDENTIFIER compatible with subinterpreters.

See also bpo-40521 for caches like free lists and Unicode interned strings.
msg379390 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2020-10-23 00:42
New changeset 345cd37abe324ad4f60f80e2c3133b8849e54e9b by Eric Snow in branch 'master':
bpo-36876: Fix the C analyzer tool. (GH-22841)
https://github.com/python/cpython/commit/345cd37abe324ad4f60f80e2c3133b8849e54e9b
msg379999 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2020-10-30 21:46
New changeset 4fe72090deb7fb7bc09bfa56c92f6b3b0967d395 by Eric Snow in branch 'master':
bpo-36876: Small adjustments to the C-analyzer tool. (GH-23045)
https://github.com/python/cpython/commit/4fe72090deb7fb7bc09bfa56c92f6b3b0967d395
msg381511 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2020-11-20 22:39
New changeset 9f02b479e6b6b48d0c2aad621978cff82e530b15 by Eric Snow in branch 'master':
bpo-36876: [c-analyzer tool] Tighten up the results and output. (GH-23431)
https://github.com/python/cpython/commit/9f02b479e6b6b48d0c2aad621978cff82e530b15
History
Date User Action Args
2020-11-20 22:39:37eric.snowsetmessages: + msg381511
2020-11-20 21:52:28eric.snowsetpull_requests: + pull_request22322
2020-10-30 21:46:55eric.snowsetmessages: + msg379999
2020-10-30 20:53:12eric.snowsetpull_requests: + pull_request21964
2020-10-23 00:42:58eric.snowsetmessages: + msg379390
2020-10-21 00:18:03eric.snowsetpull_requests: + pull_request21795
2020-05-15 01:41:00db3lsetnosy: - db3l
2020-05-15 01:37:33vstinnersetmessages: + msg368910
2020-05-15 00:37:07vstinnersetcomponents: + Subinterpreters, - Interpreter Core
title: Global C variables are a problem. -> [subinterpreters] Global C variables are a problem
2020-02-07 15:21:02maciej.szuliksetnosy: + maciej.szulik
2019-11-23 00:43:11eric.snowsetmessages: + msg357353
2019-11-23 00:42:18eric.snowsetmessages: + msg357352
2019-11-23 00:41:07eric.snowsetmessages: + msg357351
2019-11-07 16:01:05yan12125setnosy: - yan12125
2019-11-07 10:09:19vinay.sajipsetmessages: + msg356181
2019-10-19 02:00:07eric.snowsetmessages: + msg354928
2019-10-18 21:07:27eric.snowsetpull_requests: + pull_request16393
2019-09-22 04:41:55vinay.sajipsetpull_requests: + pull_request15904
2019-09-17 16:37:28vinay.sajipsetnosy: + vinay.sajip
2019-09-12 15:59:41eric.snowsetmessages: + msg352208
2019-09-12 14:41:00eric.snowsetpull_requests: + pull_request15681
2019-09-12 09:51:03eric.snowsetmessages: + msg352085
2019-09-12 09:23:14eric.snowsetpull_requests: + pull_request15640
2019-09-12 09:03:11eric.snowsetmessages: + msg352063
2019-09-12 03:43:33yan12125setnosy: + yan12125
2019-09-11 23:37:17db3lsetnosy: + db3l
messages: + msg352032
2019-09-11 18:49:48eric.snowsetmessages: + msg352013
2019-09-10 15:31:41eric.snowsetpull_requests: + pull_request15518
2019-09-09 11:30:34eric.snowsetpull_requests: + pull_request15413
2019-08-22 02:18:01phsilvasetnosy: + phsilva
2019-05-23 22:55:58eric.snowsetpull_requests: + pull_request13445
2019-05-17 00:46:59eric.snowsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request13283
2019-05-11 16:09:44christian.heimessetnosy: + christian.heimes
2019-05-10 22:19:01pablogsalsetnosy: + pablogsal
2019-05-10 20:13:09eric.snowsetmessages: + msg342125
2019-05-10 19:01:42eric.snowcreate