Skip to content

Document os.environ[x] = y and os.putenv() as thread unsafe #83556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gpshead opened this issue Jan 18, 2020 · 5 comments
Open

Document os.environ[x] = y and os.putenv() as thread unsafe #83556

gpshead opened this issue Jan 18, 2020 · 5 comments
Labels
3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@gpshead
Copy link
Member

gpshead commented Jan 18, 2020

BPO 39375
Nosy @gpshead, @eryksun, @nanjekyejoannah

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:36:07.918>
labels = ['type-feature', '3.8', '3.9', '3.10', 'docs']
title = 'Document os.environ[x] = y and os.putenv() as thread unsafe'
updated_at = <Date 2021-03-04.18:23:52.578>
user = 'https://github.com/gpshead'

bugs.python.org fields:

activity = <Date 2021-03-04.18:23:52.578>
actor = 'eryksun'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation']
creation = <Date 2020-01-18.00:36:07.918>
creator = 'gregory.p.smith'
dependencies = []
files = []
hgrepos = []
issue_num = 39375
keywords = []
message_count = 5.0
messages = ['360221', '360223', '360450', '360451', '375992']
nosy_count = 5.0
nosy_names = ['gregory.p.smith', 'docs@python', 'eryksun', 'nanjekyejoannah', 'Daniel Martin']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue39375'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

@gpshead
Copy link
Member Author

gpshead commented Jan 18, 2020

The underlying API calls made by os.putenv() and os.environ[name] = value syntax are not thread safe on POSIX systems. POSIX _does not have_ any thread safe way to access the process global environment.

In a pure Python program, the GIL prevents this from being an issue. But when embedded in a C/C++ program or using extension modules that launch their own threads from C, those threads could also make the invalid assumption that they can safely _read_ the environment. Which is a race condition when a Python thread is doing a putenv() at the same time.

We should document the danger.

CPython's os module snapshots a copy of the environment into a dict at import time (during CPython startup). But os.environ[] assignment and os.putenv() modify the actual process global environment in addition to updating this dict. (If an embedded interpreter is launched from a process with other threads already running, even that initial environment reading could be unsafe if the larger application has a thread that wrongly assumes it has exclusive environment access)

For people modifying os.environ so that the change is visible to child processes, we can recommend using the env= parameter on subprocess API calls to supply a new environment.

A broader issue of should we be modifying the process global environment state at all from os.putenv() and os.environ[] assignment still exists. I'll track that in another issue (to be opened).

@gpshead gpshead added 3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes labels Jan 18, 2020
@gpshead gpshead added 3.7 (EOL) end of life docs Documentation in the Doc dir 3.8 (EOL) end of life 3.9 only security fixes labels Jan 18, 2020
@gpshead gpshead added the docs Documentation in the Doc dir label Jan 18, 2020
@gpshead
Copy link
Member Author

gpshead commented Jan 18, 2020

https://bugs.python.org/issue39376 tracks possible interpreter behavior changes.

@gpshead
Copy link
Member Author

gpshead commented Jan 22, 2020

fwiw, no need to remove that message. We'll want to make the docs clear that this does not apply to Windows. :)

@eryksun
Copy link
Contributor

eryksun commented Jan 22, 2020

no need to remove that message.

I was discussing the wrong API. It's not directly relevant that Windows API functions that access the process environment are protected by the PEB lock. Python primarily uses [_w]environ, a copy of the process environment that's managed by the C runtime library (ucrt). os.putenv modifies this environment via _wputenv. (Ultimately this syncs with the process environment by calling SetEnvironmentVariableW.)

Functions that modify and read ucrt's environment are protected by a lock. But there's still a concern if multithreaded code reads or modifies [_w]environ concurrent to a _[w]putenv call. Also, [_w]getenv returns a pointer to the value in [_w]environ, so it has the same problem. A significant difference, however, is that _[w]putenv in ucrt is not POSIX compliant, since it copies the caller's string. Also, ucrt has safer [_w]getenv_s and _[w]dupenv_s functions that return a copy.

@DanielMartin
Copy link
Mannequin

DanielMartin mannequin commented Aug 27, 2020

See also https://bugs.python.org/issue40961 - that bug is not about thread safety, but another quirk around env. variable setting that needs to be documented in the documentation for os.putenv and os.getenv, and so anyone addressing this bug should probably address that one at the same time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement
Projects
Status: No status
Development

No branches or pull requests

2 participants