This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: environment variables not passed correctly using new virtualenv launching in windows and python3.7+
Type: crash Stage: resolved
Components: Windows Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eryksun, miss-islington, paul.moore, pierreglaser, pitrou, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2019-09-10 13:32 by pierreglaser, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15883 closed pierreglaser, 2019-09-10 16:06
PR 16098 merged steve.dower, 2019-09-13 13:12
PR 16116 merged miss-islington, 2019-09-13 16:40
PR 18157 merged meilyadam, 2020-01-24 03:06
Messages (12)
msg351650 - (view) Author: Pierre Glaser (pierreglaser) * Date: 2019-09-10 13:32
If I am not mistaken, when creating a new process on Python3.7 and later on Windows, if using a virtualenv, Python now uses a launcher. The launcher is being notified that it must create a virtual-environment Python (and not a system Python) program using the __PYVENV_LAUNCHER__ environment variable, passed as part of the environment of launcher process created using in _winapi.CreateProcess
(see https://github.com/python/cpython/blob/9008be303a89bfab8c3314c6a42330b5523adc8b/Lib/multiprocessing/popen_spawn_win32.py#L73-L75)

However, if I am not mistaken `env` is not passed at the right position (https://github.com/python/cpython/blob/9008be303a89bfab8c3314c6a42330b5523adc8b/Modules/_winapi.c#L1062-L1068). These lines were part of a bugfix patch (see https://bugs.python.org/issue35797), solving an issue for multiprocessing-based packages. We ended trying to backport to loky (https://github.com/tomMoral/loky, a multiprocessing-based package) but the bug was not fixed. Passing 'env' correctly fixed the bug.

Two things:
- It is hard to provide a reproducer for this issue as it requires patching the CPython source code.
- I don't understand why env being not passed correctly does not manifest itself in the test-suite. What is even more troubling is that even with this bug, the virtualenv launcher seems to get that a virtualenv is used when creating processes in multiprocessing (at least, sys.path includes the path/to/venv/lib/python3.x/site-packages). However, I am not familiar with the virtualenv launcher for python3.7+ and windows.
msg351670 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-09-10 14:21
Yeah, very strange that. I can only assume that it's launching the venv redirector directly, rather than the real sys.executable, and we aren't ever calling set_executable() with the real one anymore.

Dropping this into Lib/multiprocessing/spawn.py should cause a repro:

if WINSERVICE:
    _python_exe = os.path.join(sys.exec_prefix, 'python.exe')
else:
    _python_exe = getattr(sys, '_base_executable', sys.executable)

And as you point out, fixing the CreateProcess call should provide a fix.

Could you try that? And maybe submit a PR with the fix?
msg351711 - (view) Author: Pierre Glaser (pierreglaser) * Date: 2019-09-10 15:52
> Dropping this into Lib/multiprocessing/spawn.py should cause a repro:

  if WINSERVICE:
      _python_exe = os.path.join(sys.exec_prefix, 'python.exe')
  else:
      _python_exe = getattr(sys, '_base_executable', sys.executable)

In this case, spawn.get_executable() will return (sys._base_executable), and `env` will be set to None anyways no? (see these lines: https://github.com/python/cpython/blob/9008be303a89bfab8c3314c6a42330b5523adc8b/Lib/multiprocessing/popen_spawn_win32.py#L59-L68)

We need to trigger the if clause of these lines instead, which happens by default in a virtual env -- this is why it is so troubling: even though a very simple case (launching a new process from within a virtualenv) should trigger a bug, it does not.

> And maybe submit a PR with the fix?

Will do.
msg351722 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-09-10 16:13
The difference is that launching sys._base_executable *without* __PYVENV_LAUNCHER__ set (because env is not being passed) should lose you access to anything installed into the venv. You may also need to import something from the venv in order to see the issue.

Launching sys.executable will hit the launcher that sets the environment variable. Setting the environment correctly and launching sys._base_executable will also load correctly. The latter is theoretically required for correct handle sharing, but that may depend on which Windows version you're running.

I'd like to see both changes in the PR. Just setting the environment variable doesn't really improve the situation at all.
msg352316 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-09-13 13:14
I posted a second PR with the rest of the change, as it'd be good to get this in before the next 3.8 release.
msg352358 - (view) Author: miss-islington (miss-islington) Date: 2019-09-13 16:40
New changeset f2b7556ef851ac85e7cbf189d1b29fdeb9539b88 by Miss Islington (bot) (Steve Dower) in branch 'master':
bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual environment (GH-16098)
https://github.com/python/cpython/commit/f2b7556ef851ac85e7cbf189d1b29fdeb9539b88
msg352363 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-09-13 16:43
Thanks for the report and partial patch!
msg352368 - (view) Author: miss-islington (miss-islington) Date: 2019-09-13 16:59
New changeset 436b429ade87b10879b3f944e99a515478e86e5e by Miss Islington (bot) in branch '3.8':
bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual environment (GH-16098)
https://github.com/python/cpython/commit/436b429ade87b10879b3f944e99a515478e86e5e
msg355275 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-10-24 02:15
This should revert to setting `_python_exe = sys.executable` in Lib/multiprocessing/spawn.py. Then the code in Lib/multiprocessing/popen_spawn_win32.py will set __PYVENV_LAUNCHER__ in the spawned process to the virtual environment's sys.executable. Otherwise the worker process has no connection to the virtual environment, other than how sys.path gets manually copied from the parent process. In particular, without setting __PYVENV_LAUNCHER__, sys.executable is not the virtual-environment executable and sys.prefix is not the virtual-environment directory.

Also, the fix for the parameters that are passed to _winapi.CreateProcess needs to be backported to 3.7. Else __PYVENV_LAUNCHER__ won't actually be set in the child.
msg355344 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-24 17:49
You're right, the logic to actually launch _base_executable is in there twice now, and the second one (that never gets used) is more important.
msg360860 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-01-28 10:29
New changeset 6990d1b6131873c7f0913908162e4c723d00ea19 by Steve Dower (Adam Meily) in branch '3.7':
bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual environment (GH-16098)
https://github.com/python/cpython/commit/6990d1b6131873c7f0913908162e4c723d00ea19
msg360863 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-01-28 10:36
Fixed via issue39439
History
Date User Action Args
2022-04-11 14:59:20adminsetgithub: 82273
2020-01-28 10:36:41steve.dowersetstatus: open -> closed
resolution: fixed
messages: + msg360863

stage: patch review -> resolved
2020-01-28 10:29:20steve.dowersetmessages: + msg360860
2020-01-24 03:06:24meilyadamsetstage: needs patch -> patch review
pull_requests: + pull_request17543
2020-01-23 22:39:03eryksunlinkissue39439 superseder
2019-10-24 17:49:08steve.dowersetmessages: + msg355344
2019-10-24 02:15:37eryksunsetstatus: closed -> open

versions: + Python 3.7
nosy: + eryksun

messages: + msg355275
resolution: fixed -> (no value)
stage: resolved -> needs patch
2019-09-13 16:59:14miss-islingtonsetmessages: + msg352368
2019-09-13 16:43:35steve.dowersetstatus: open -> closed
resolution: fixed
messages: + msg352363

stage: patch review -> resolved
2019-09-13 16:40:30miss-islingtonsetpull_requests: + pull_request15731
2019-09-13 16:40:23miss-islingtonsetnosy: + miss-islington
messages: + msg352358
2019-09-13 13:14:22steve.dowersetmessages: + msg352316
versions: - Python 3.7
2019-09-13 13:12:34steve.dowersetpull_requests: + pull_request15718
2019-09-10 16:13:26steve.dowersetmessages: + msg351722
2019-09-10 16:06:27pierreglasersetkeywords: + patch
stage: patch review
pull_requests: + pull_request15524
2019-09-10 15:52:15pierreglasersetmessages: + msg351711
2019-09-10 14:21:23steve.dowersetmessages: + msg351670
2019-09-10 13:32:39pierreglasercreate