bpo-39395: Clear env vars set by Python at exit by vstinner · Pull Request #18078 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation7 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

vstinner

Environment variables set by os.environ and os.putenv() are now
cleared at exit. Python manages their memory which is released
at exit.

https://bugs.python.org/issue39395

@vstinner

Environment variables set by os.environ and os.putenv() are now cleared at exit. Python manages their memory which is released at exit.

@vstinner

eduardo-elizondo

static int
_posix_clear(PyObject *module)
{
Py_CLEAR(_posixstate(module)->billion);
#ifdef HAVE_UNSETENV
posix_unset_envvars(_posixstate(module)->posix_putenv_garbage);
#endif
Py_CLEAR(_posixstate(module)->posix_putenv_garbage);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that you are already doing these fixes around this, posix_putenv_garbage should also be guarded by HAVE_PUTENV (check its initialization in the init function).

@eduardo-elizondo

Good catch, looks good to me 👍

eduardo-elizondo

@ned-deily

Should the docs for os.environ and os.putenv be updated to note this explicit deletion? Is it possible that, on some platforms, existing programs might be using the current undefined behavior without crashing and thus might break with this change so it should be documented?

serhiy-storchaka

@vstinner

Should the docs for os.environ and os.putenv be updated to note this explicit deletion? Is it possible that, on some platforms, existing programs might be using the current undefined behavior without crashing and thus might break with this change so it should be documented?

Yes, it is possible. Python 3.8 was safe: the memory was never released, so environment variables remained available and didn't crash.

I will first implement https://bugs.python.org/issue39406 before updating this PR to document the incompatible change.

@vstinner

I will first implement https://bugs.python.org/issue39406 before updating this PR to document the incompatible change.

This issue is now implemented: os.putenv() is now implemented with setenv() if setenv() is available.

So I proposed PR #18135 to fix bpo-39395 differently: on non-Windows platforms, Python now requires setenv() to build. This PR has no backward compatibility issue: it doesn't unset environment variables at exit.

@vstinner

I wrote PR #18135 which doesn't introduce a backward incompatible change. I close this PR.