search

python / cpython

The Python programming language
https://www.python.org
Other
63.67k stars 30.51k forks source link

Do not add duplicate FDs to list in duplicate_for_child() #87308

Open ba7f5cf3-d13d-40c6-88f9-6439f38ff0a0 opened 3 years ago

ba7f5cf3-d13d-40c6-88f9-6439f38ff0a0 commented 3 years ago
BPO 43142
Nosy @terryjreedy, @pitrou, @applio, @imaginary-person
PRs
  • python/cpython#24461
    		•

    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: ```python assignee = None closed_at = None created_at = labels = ['3.8', 'type-bug', 'library', '3.9', '3.10'] title = 'Do not add duplicate FDs to list in duplicate_for_child()' updated_at = user = 'https://github.com/imaginary-person' ``` bugs.python.org fields: ```python activity = actor = 'terry.reedy' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'sanchit' dependencies = [] files = [] hgrepos = [] issue_num = 43142 keywords = ['patch'] message_count = 4.0 messages = ['386546', '386881', '386897', '386913'] nosy_count = 4.0 nosy_names = ['terry.reedy', 'pitrou', 'davin', 'sanchit'] pr_nums = ['24461'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue43142' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10'] ```

    ba7f5cf3-d13d-40c6-88f9-6439f38ff0a0 commented 3 years ago

    While spawning, if a process having code built on top of Python's multiprocessing module calls multiprocessing.reduction.DupFd() (in /Lib/multiprocessing/reduction.py) multiple times on the same File descriptor by mistake, then it ends up invoking duplicate_for_child(fd)) of the class Popen, which, in turn, adds a duplicate FD to a list. This list is then used in spawnv_passfds() in /Lib/multiprocessing/util.py, which uses that list as an argument in a call of _posixsubprocess.fork_exec().

    In Modules/_posixsubprocess.c, checks exist to ensure that all the FDs in the list are unique, positive, and in a sorted order. If duplicate entries are found, a ValueError: bad value(s) in fds_to_keep exception is raised, and the execution is terminated. Even if this exception were somehow handled, a developer can't change the aforementioned FD list in Python without breaking modularity.

    It's better to let developers get away with calling multiprocessing.reduction.DupFd() (in /Lib/multiprocessing/reduction.py) on by not accepting duplicate entries in the first place, so that spawning a process can be successful.

    terryjreedy commented 3 years ago

    3.7 and before only get security fixes.

    ba7f5cf3-d13d-40c6-88f9-6439f38ff0a0 commented 3 years ago

    But can you please add this fix in versions 3.8 & beyond? Thanks!

    From: report=bugs.python.org@roundup.psfhosted.org \report=bugs.python.org@roundup.psfhosted.org\ on behalf of Terry J. Reedy \report@bugs.python.org\ Sent: Friday, February 12, 2021 3:43 PM To: SANCHIT JAIN \sanchit@cs.wisc.edu\ Subject: [bpo-43142] Do not add duplicate FDs to list in duplicate_for_child()

    Terry J. Reedy \tjreedy@udel.edu\ added the comment:

    3.7 and before only get security fixes.

    ---------- nosy: +davin, pitrou, terry.reedy versions: -Python 3.6, Python 3.7

    Python tracker \report@bugs.python.org\ \https://bugs.python.org/issue43142\

    terryjreedy commented 3 years ago

    I can't (lacking multiprocessing knowledge), but someone else might.

    PS: when replying via email, please delete post you are responding to. When added to the web page, the copy is redundant noise.