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

bpo-39395: Clear env vars set by Python at exit #18078

Closed
wants to merge 1 commit into from
Closed

bpo-39395: Clear env vars set by Python at exit #18078

wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jan 20, 2020

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

Environment variables set by os.environ and os.putenv() are now
cleared at exit. Python manages their memory which is released
at exit.
@vstinner
Copy link
Member Author

cc @eduardo-elizondo @ericsnowcurrently @serhiy-storchaka @encukou (author and reviewers of PR #10854 and PR #15892).

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);
Copy link
Contributor

@eduardo-elizondo eduardo-elizondo Jan 20, 2020

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
Copy link
Contributor

Good catch, looks good to me 👍

@ned-deily
Copy link
Member

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?

@vstinner
Copy link
Member Author

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.

@serhiy-storchaka serhiy-storchaka added needs backport to 3.7 type-bug An unexpected behavior, bug, or error labels Jan 22, 2020
@vstinner
Copy link
Member Author

vstinner commented Jan 23, 2020

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
Copy link
Member Author

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

@vstinner vstinner closed this Jan 24, 2020
@vstinner vstinner deleted the posix_unset_envvars branch January 24, 2020 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting merge type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants