environment variables not passed correctly using new virtualenv launching in windows and python3.7+ #82273

pierreglaser · 2019-09-10T13:32:40Z

BPO	38092
Nosy	@pfmoore, @pitrou, @tjguk, @zware, @eryksun, @zooba, @miss-islington, @pierreglaser
PRs	bpo-38092: Correctly pass __PYVENV_LAUNCHER__ in when creating processes in windows #15883 bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual environment #16098 [3.8] bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual environment (GH-16098) #16116 [3.7] bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual environment (GH-16098) #18157

^{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 = <Date 2020-01-28.10:36:41.181>
created_at = <Date 2019-09-10.13:32:39.880>
labels = ['3.7', '3.8', 'OS-windows', 'type-crash', '3.9']
title = 'environment variables not passed correctly using new virtualenv launching in windows and python3.7+'
updated_at = <Date 2020-01-28.10:36:41.180>
user = 'https://github.com/pierreglaser'

bugs.python.org fields:

activity = <Date 2020-01-28.10:36:41.180>
actor = 'steve.dower'
assignee = 'none'
closed = True
closed_date = <Date 2020-01-28.10:36:41.181>
closer = 'steve.dower'
components = ['Windows']
creation = <Date 2019-09-10.13:32:39.880>
creator = 'pierreglaser'
dependencies = []
files = []
hgrepos = []
issue_num = 38092
keywords = ['patch']
message_count = 12.0
messages = ['351650', '351670', '351711', '351722', '352316', '352358', '352363', '352368', '355275', '355344', '360860', '360863']
nosy_count = 8.0
nosy_names = ['paul.moore', 'pitrou', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'miss-islington', 'pierreglaser']
pr_nums = ['15883', '16098', '16116', '18157']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue38092'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

pierreglaser · 2019-09-10T13:32:40Z

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

cpython/Lib/multiprocessing/popen_spawn_win32.py

Lines 73 to 75 in 9008be3

    
           hp, ht, pid, tid = _winapi.CreateProcess( 
        
               python_exe, cmd, 
        
               env, None, False, 0, None, None, None)

)

However, if I am not mistaken env is not passed at the right position (

cpython/Modules/_winapi.c

Lines 1062 to 1068 in 9008be3

    
           _winapi_CreateProcess_impl(PyObject *module, 
        
                                      const Py_UNICODE *application_name, 
        
                                      PyObject *command_line, PyObject *proc_attrs, 
        
                                      PyObject *thread_attrs, BOOL inherit_handles, 
        
                                      DWORD creation_flags, PyObject *env_mapping, 
        
                                      const Py_UNICODE *current_directory, 
        
                                      PyObject *startup_info)

). 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.

zooba · 2019-09-10T14:21:23Z

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?

pierreglaser · 2019-09-10T15:52:16Z

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:

cpython/Lib/multiprocessing/popen_spawn_win32.py

Lines 59 to 68 in 9008be3

    
           python_exe = spawn.get_executable() 
        
           # bpo-35797: When running in a venv, we bypass the redirect 
        
           # executor and launch our base Python. 
        
           if WINENV and _path_eq(python_exe, sys.executable): 
        
               python_exe = sys._base_executable 
        
               env = os.environ.copy() 
        
               env["__PYVENV_LAUNCHER__"] = sys.executable 
        
           else: 
        
               env = None

)

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.

zooba · 2019-09-10T16:13:27Z

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.

zooba · 2019-09-13T13:14:23Z

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.

miss-islington · 2019-09-13T16:40:23Z

New changeset f2b7556 by Miss Islington (bot) (Steve Dower) in branch 'master':
bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual environment (GH-16098)
f2b7556

zooba · 2019-09-13T16:43:35Z

Thanks for the report and partial patch!

miss-islington · 2019-09-13T16:59:14Z

New changeset 436b429 by Miss Islington (bot) in branch '3.8':
bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual environment (GH-16098)
436b429

eryksun · 2019-10-24T02:15:38Z

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.

zooba · 2019-10-24T17:49:08Z

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.

zooba · 2020-01-28T10:29:20Z

New changeset 6990d1b by Steve Dower (Adam Meily) in branch '3.7':
bpo-38092: Reduce overhead when using multiprocessing in a Windows virtual environment (GH-16098)
6990d1b

zooba · 2020-01-28T10:36:41Z

Fixed via bpo-39439

pierreglaser mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes OS-windows type-crash A hard crash of the interpreter, possibly with a core dump labels Sep 10, 2019

zooba removed the 3.7 (EOL) end of life label Sep 13, 2019

zooba closed this as completed Sep 13, 2019

eryksun added the 3.7 (EOL) end of life label Oct 24, 2019

eryksun reopened this Oct 24, 2019

zooba closed this as completed Jan 28, 2020

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

environment variables not passed correctly using new virtualenv launching in windows and python3.7+ #82273

environment variables not passed correctly using new virtualenv launching in windows and python3.7+ #82273

pierreglaser mannequin commented Sep 10, 2019

pierreglaser mannequin commented Sep 10, 2019

zooba commented Sep 10, 2019

pierreglaser mannequin commented Sep 10, 2019

zooba commented Sep 10, 2019

zooba commented Sep 13, 2019

miss-islington commented Sep 13, 2019

zooba commented Sep 13, 2019

miss-islington commented Sep 13, 2019

eryksun commented Oct 24, 2019

zooba commented Oct 24, 2019

zooba commented Jan 28, 2020

zooba commented Jan 28, 2020

environment variables not passed correctly using new virtualenv launching in windows and python3.7+ #82273

environment variables not passed correctly using new virtualenv launching in windows and python3.7+ #82273

Comments

pierreglaser mannequin commented Sep 10, 2019

pierreglaser mannequin commented Sep 10, 2019

zooba commented Sep 10, 2019

pierreglaser mannequin commented Sep 10, 2019

zooba commented Sep 10, 2019

zooba commented Sep 13, 2019

miss-islington commented Sep 13, 2019

zooba commented Sep 13, 2019

miss-islington commented Sep 13, 2019

eryksun commented Oct 24, 2019

zooba commented Oct 24, 2019

zooba commented Jan 28, 2020

zooba commented Jan 28, 2020