Skip to content
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

subprocess.Popen can pass incomplete environment when posix_spawn is used #113119

Closed
kulikjak opened this issue Dec 14, 2023 · 2 comments · Fixed by #113120
Closed

subprocess.Popen can pass incomplete environment when posix_spawn is used #113119

kulikjak opened this issue Dec 14, 2023 · 2 comments · Fixed by #113120
Assignees
Labels
type-bug An unexpected behavior, bug, or error

Comments

@kulikjak
Copy link
Contributor

kulikjak commented Dec 14, 2023

Bug report

Bug description:

Subprocess created with posix_spawn can have different environment when compared to a process created with stardard fork/exec.

The reason is that posix_spawn uses os.environ (when no env was given), which is not affected by os.putenv and os.unsetenv. This is different from execv, which uses process environment (available through environ).

This can be easily reproduced with test_putenv_unsetenv modified to call subprocess.run with close_fds=False (which makes it use the posix_spawn call):

import subprocess
import sys
import os

name = "PYTHONTESTVAR"
value = "testvalue"
code = f'import os; print(repr(os.environ.get({name!r})))'

os.putenv(name, value)
proc = subprocess.run([sys.executable, '-c', code], check=True,
                      stdout=subprocess.PIPE, text=True, close_fds=False)
assert proc.stdout.rstrip() == repr(value)

os.unsetenv(name)
proc = subprocess.run([sys.executable, '-c', code], check=True,
                      stdout=subprocess.PIPE, text=True, close_fds=False)
assert proc.stdout.rstrip() == repr(None)

I found it when I patched Python with #113118 (on Solaris, but this is pretty platform independent, I think).

CPython versions tested on:

3.9, 3.11

Operating systems tested on:

Other

Linked PRs

@kulikjak kulikjak added the type-bug An unexpected behavior, bug, or error label Dec 14, 2023
@gpshead gpshead self-assigned this Dec 15, 2023
gpshead pushed a commit that referenced this issue Dec 17, 2023
…wn is used (#113120)

* Allow posix_spawn to inherit environment form parent environ variable.

With this change, posix_spawn call can behave similarly to execv with regards to environments when used in subprocess functions.
@gpshead gpshead reopened this Dec 19, 2023
@gpshead
Copy link
Member

gpshead commented Dec 19, 2023

reopened due to @ned-deily reporting on a parallel issue #113117 about this change:

This breaks macOS framework builds such as those used by the python.org installer:

$ ./configure --enable-framework && make -j
[...]
./Modules/posixmodule.c:7170:19: error: use of undeclared identifier 'environ'
        envlist = environ;
                  ^
./Modules/posixmodule.c:7238:31: error: use of undeclared identifier 'environ'
    if (envlist && envlist != environ) {
                              ^

Note the comments elsewhere in posixmodule.c such as at line 1582:

#elif defined(WITH_NEXT_FRAMEWORK) || (defined(__APPLE__) && defined(Py_ENABLE_SHARED))
    /* environ is not accessible as an extern in a shared object on OSX; use
       _NSGetEnviron to resolve it. The value changes if you add environment
       variables between calls to Py_Initialize, so don't cache the value. */
    e = *_NSGetEnviron();

gpshead added a commit that referenced this issue Dec 19, 2023
`--enable-framework` builds were failing.  we apparently do not have good CI & buildbot coverage here.
@gpshead gpshead closed this as completed Dec 19, 2023
@kulikjak
Copy link
Contributor Author

Sorry for that, and thank you for the fix. Since CI was green, I didn't look into it much more and missed that.

ryan-duve pushed a commit to ryan-duve/cpython that referenced this issue Dec 26, 2023
`--enable-framework` builds were failing.  we apparently do not have good CI & buildbot coverage here.
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
…ix_spawn is used (python#113120)

* Allow posix_spawn to inherit environment form parent environ variable.

With this change, posix_spawn call can behave similarly to execv with regards to environments when used in subprocess functions.
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
`--enable-framework` builds were failing.  we apparently do not have good CI & buildbot coverage here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants