[subinterpreters] Global C variables are a problem

[ericsnowcurrently]

BPO	36876
Nosy	@vsajip, @vstinner, @tiran, @phsilva, @ericsnowcurrently, @soltysh, @pablogsal, @sweeneyde
PRs	bpo-36876: Avoid static locals. #13372 bpo-36876: Use a consistent variable name for kwlist. #13531 bpo-36876: Make some static string literal arrays constant. #15760 bpo-36876: Add a tool that identifies unsupported global C variables. #15877 bpo-36876: Skip test_check_c_globals for now. #16017 bpo-36876: Fix the globals checker tool. #16058 bpo-36876: Moved Parser/listnode.c statics to interpreter state. #16328 bpo-36876: Re-organize the c-analyzer tool code. #16841 bpo-36876: Fix the C analyzer tool. #22841 bpo-36876: Small adjustments to the C-analyzer tool. #23045 bpo-36876: [c-analyzer tool] Tighten up the results and output. #23431 bpo-36876: [c-analyzer tool] Add a "capi" subcommand to the c-analyzer tool. #23918 bpo-36876: [c-analyzer tool] Additional CLI updates for "capi" command. #23929 bpo-36876: Update the c-analyzer whitelist. #31225 bpo-36876: Minor cleanup to c-analyzer "ignored" data.' #31239 bpo-36876: Make sure the c-analyzer is checking all the source files.' #31264

^{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 = 'https://github.com/ericsnowcurrently'
closed_at = None
created_at = <Date 2019-05-10.19:01:42.087>
labels = ['expert-subinterpreters', 'type-bug', '3.8']
title = '[subinterpreters] Global C variables are a problem'
updated_at = <Date 2022-02-10.23:14:23.025>
user = 'https://github.com/ericsnowcurrently'

bugs.python.org fields:

activity = <Date 2022-02-10.23:14:23.025>
actor = 'eric.snow'
assignee = 'eric.snow'
closed = False
closed_date = None
closer = None
components = ['Subinterpreters']
creation = <Date 2019-05-10.19:01:42.087>
creator = 'eric.snow'
dependencies = []
files = []
hgrepos = []
issue_num = 36876
keywords = ['patch']
message_count = 22.0
messages = ['342119', '342125', '352013', '352032', '352063', '352085', '352208', '354928', '356181', '357351', '357352', '357353', '368910', '379390', '379999', '381511', '383698', '383778', '393785', '412886', '412957', '413029']
nosy_count = 8.0
nosy_names = ['vinay.sajip', 'vstinner', 'christian.heimes', 'phsilva', 'eric.snow', 'maciej.szulik', 'pablogsal', 'Dennis Sweeney']
pr_nums = ['13372', '13531', '15760', '15877', '16017', '16058', '16328', '16841', '22841', '23045', '23431', '23918', '23929', '31225', '31239', '31264']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36876'
versions = ['Python 3.8']

• PR: gh-81057: Move the Allocators to _PyRuntimeState #99217

• PR: gh-81057: Move the Extension Modules Cache to _PyRuntimeState #99355

• PR: gh-81057: Generate a Separate Initializer For Each Part of the Global Objects Initializer #99389

• PR: gh-81057: Add PyInterpreterState.static_objects #99397

• PR: gh-81057: Move contextvars-related Globals to _PyRuntimeState #99400

• PR: gh-81057: Move PyImport_Inittab to _PyRuntimeState #99402

• PR: GH-81057: remove static state from arraymodule #99409

• PR: GH-81057: remove static state from suggestions.c #99411

• PR: gh-81057: Move Global Variables Holding Objects to _PyRuntimeState. #99487

• PR: gh-81057: Move the Remaining Import State Globals to _PyRuntimeState #99488

• PR: gh-81057: Move Globals in Core Code to _PyRuntimeState #99496

• PR: gh-81057: Move the global Dict-Related Versions to _PyRuntimeState #99497

• PR: gh-81057: Move More Globals in Core Code to _PyRuntimeState #99516

• PR: gh-81057: Move OS-Related Globals to _PyRuntimeState #100082

• PR: gh-81057: Move Ceval Trampoline Globals to _PyRuntimeState #100083

• PR: gh-81057: Move Threading-Related Globals to _PyRuntimeState #100084

• PR: gh-81057: Move Signal-Related Globals to _PyRuntimeState #100085

• PR: gh-81057: Move More Globals to _PyRuntimeState #100092

• PR: gh-81057: Move time Globals to _PyRuntimeState #100122

• PR: gh-81057: Fix an ifdef in the time module #100125

• PR: gh-81057: Fix the wasm32-wasi Buildbot #100139

• PR: gh-81057: Fix a Reference Leak in the posix Module #100140

• PR: gh-81057: Move tracemalloc Globals to _PyRuntimeState #100151

• PR: gh-81057: Move faulthandler Globals to _PyRuntimeState #100152

• PR: gh-81057: Move _Py_RefTotal to the "Ignored Globals" List #100202

• PR: gh-81057: Move _Py_RefTotal to the "Ignored Globals" List #100203

• PR: gh-81057: Move the Cached Parser Dummy Name to _PyRuntimeState #100277

• PR: gh-81057: Vendor a Subset of distutils for the c-analyzer Tool #102505

• PR: gh-81057: Add a CI Check for New Unsupported C Global Variables #102506

• PR: gh-81057: Inline single-use error messages in _decimal and make the rest const #156809

[ericsnowcurrently]

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.

[ericsnowcurrently]

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.

[ericsnowcurrently]

New changeset ee536b2 by Eric Snow in branch 'master':
bpo-36876: Add a tool that identifies unsupported global C variables. (bpo-15877)
ee536b2

[db3l]

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.

[ericsnowcurrently]

@db3l, I'll take a look right away.

[ericsnowcurrently]

New changeset 64535fc by Eric Snow in branch 'master':
bpo-36876: Skip test_check_c_globals for now. (gh-16017)
64535fc

[ericsnowcurrently]

New changeset 088b63e by Eric Snow in branch 'master':
bpo-36876: Fix the globals checker tool. (gh-16058)
088b63e

[ericsnowcurrently]

New changeset e4c431e by Eric Snow in branch 'master':
bpo-36876: Re-organize the c-analyzer tool code. (gh-16841)
e4c431e

[vsajip]

New changeset 9def81a by Vinay Sajip in branch 'master':
bpo-36876: Moved Parser/listnode.c statics to interpreter state. (GH-16328)
9def81a

[ericsnowcurrently]

Thanks, Vinay!

[ericsnowcurrently]

FYI, others have been tackling this in separate issues (e.g. Victor, anyone relative to PEP-384).