gh-144503: Pass sys.argv via a route less limited by single command line argument length
#144508
Conversation
The maximum length of a single command line argument is more restricted than the total size of all command line arguments together.
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
The `resource` module is not available in a WASI environment.
sjoerdjob
force-pushed
the
issue-144503-multiprocessing-forkserver-arglen
branch
from
c375314 to
0507aa0
Compare
Lib/multiprocessing/forkserver.py
Outdated
| else: | ||
| authkey = b'' | ||
|
|
||
| if preload: |
There was a problem hiding this comment.
I'm kind-of inclined to think that this logic should be in _handle_preload, but I'm not sure how to combine that with the tests that were introduced in gh-141859.
…ad of argv The allowed size for argv is limited, while the amount of data that can be sent over a pipe is virtually unlimited.
| main_kws = {} | ||
| if self._preload_modules: | ||
| data = spawn.get_preparation_data('ignore') | ||
| if 'sys_path' in data: |
There was a problem hiding this comment.
The if ... in data have been removed, because a cursory reading of get_preparation_data shows that the sys_path, sys_argv are always present in the returned dictionary.
The init_main_from_path is not always present, but the default argument in _handle_preload is None, so I think using that same value here is fine.
Similarly to on_error: this was checking if it was equal to the default value of _handle_preload, and if so, would not pass it. If the default value of _handle_preload would be changed, and this statement here would not be, that would cause an inconsistency. I think it's better to just always pass the value here to prevent future drift.
sjoerdjob
changed the title
sys.argv as separate command line arguments.sys.argv via a route less limited by single command line argument length
| exe = spawn.get_executable() | ||
| args = [exe] + util._args_from_interpreter_flags() | ||
| args += ['-c', cmd] | ||
| pid = util.spawnv_passfds(exe, args, fds_to_pass) |
There was a problem hiding this comment.
Out of curiosity, in the hope someone else knowns; is there any reason why the forkserver is spawned using util.spawnv_fds, which via a few steps, executes: _posixsubprocess.fork_exec?
The flow is now a bit similar to:
fork- creates a new child process with the same shared memory spaceexec- replaces the whole process with a new processpreload- we do our best to preload some of the modules that were requested, including__main__which we dropped when weexecd.
Now, the documentation on https://docs.python.org/3/library/os.html#os.fork is quite forceful that there are lots of problems with using os.fork(). So I guess that that is the reason to not use os.fork here in a forkserver.
1 participant
The maximum length of a single command line argument is more restricted than the total size of all command line arguments together.
multiprocessing.Poolstarts to fail if command has a long list of arguments #144503