Avoid modifying the process global environment (not thread safe)

[gpshead]

BPO	39376
Nosy	@gpshead, @ericsnowcurrently

^{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 = None
created_at = <Date 2020-01-18.00:47:44.215>
labels = ['expert-subinterpreters', 'interpreter-core', '3.9', 'type-crash']
title = 'Avoid modifying the process global environment (not thread safe)'
updated_at = <Date 2021-04-16.00:43:07.242>
user = 'https://github.com/gpshead'

bugs.python.org fields:

activity = <Date 2021-04-16.00:43:07.242>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core', 'Subinterpreters']
creation = <Date 2020-01-18.00:47:44.215>
creator = 'gregory.p.smith'
dependencies = []
files = []
hgrepos = []
issue_num = 39376
keywords = []
message_count = 2.0
messages = ['360222', '360225']
nosy_count = 2.0
nosy_names = ['gregory.p.smith', 'eric.snow']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue39376'
versions = ['Python 3.9']

[gpshead]

For more context, see https://bugs.python.org/issue39375 which seeks to document the existing caveats.

POSIX lacks any APIs to access the process global environment in a thread safe manner. Given this, we could _consider_ preventing os.putenv() and os.environ[x] = y assignment from actually modifying the process global environment. They'd save their changes in our local os.environ underlying dict, set a flag that it was modified, but not modify the global.

This would be a visible behavior change and break _some_ class of code. :/

Our stdlib codepaths that launch a new process on POSIX could be modified to to always pass our a newly constructed envp from os.environ to exec/spawn APIs. The os.system() API would need to stop using the POSIX system() API call in order for that to work.

Downside API breakage: Extension module modifications to the environment would not be picked up by Python interpreter launched subprocesses. How much of a problem would that be in practice?

We may decide to close this as infeasible and just stick with the documentation of the sorry state of POSIX and not attempt to offer any safe non-crash-possible workarounds.

[ericsnowcurrently]

+1

This has impact on subinterpreters once they stop sharing the GIL. (It's already on my list of global resources that need better protection.)

[ericsnowcurrently]

@colesbury, is this something that needs to be addressed for free-threaded builds?

[gpshead]

given modifying the C/kernel provided environment blob is already broken regardless of free-threaded builds, I doubt it needs special attention beyond making sure the existing os.putenv(x, y) and os.environ[x] = y documentation calls it out.

those APIs haven't been compatible with the modern world for decades.

[gpshead]

If we went with my idea in the opening message of tracking local modifications, it might be a challenge. But my gut feeling is that we just won't bother doing this? I don't recall the last time I've seen this come up as an issue. Hopefully that means people just don't do it and pass new env= mappings to subprocesses?

[colesbury]

I agree with what @gpshead wrote -- I think we should document it, but I don't think there's a good fix for it at the Python level.

The same problem affects other languages and runtimes:

• Go: os: Setenv leads to SIGSEGV with GODEBUG=netdns=cgo, and other uses of cgo golang/go#63567 (comment)
• Rust: Make std::env::{set_var, remove_var} unsafe in edition 2024 rust-lang/rust#124636 (marked "unsafe" recently)

Rachel Kroll had a blog post ~7 years ago about setenv that concludes with:

None of this is new, but we do re-discover it roughly every five years.
See you in 2022.

I expect we'll have users run into this problem every so often, even with updated docs. It's too bad because I think the technical fix for glibc, etc. is not too complex.

[gpshead]

While I mostly like to pretend putenv() or modifying main's char **environ doesn't exist and the world has largely adopted the same mentality... The code I added recently to try and detect a multithreaded process to warn people who were forking could be generalized and reused here for an environment modification warning when threads exist.

https://github.com/python/cpython/blob/main/Modules/posixmodule.c#L7884 warn_about_fork_with_threads()

[tbu-]

Permalink:

cpython/Modules/posixmodule.c

Line 7884 in 0890ad7

warn_about_fork_with_threads(const char* name)