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.

Sign up for GitHub

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

Jump to bottom

subprocess.py leaks fd in communicate #47040

Closed
jrosdahl mannequin opened this issue May 8, 2008 · 4 comments
Closed

subprocess.py leaks fd in communicate #47040

jrosdahl mannequin opened this issue May 8, 2008 · 4 comments
Assignees
@gpshead
Labels
performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@jrosdahl
Copy link
Mannequin

jrosdahl mannequin commented May 8, 2008

BPO 2791
Nosy @jrosdahl, @gpshead
Files
  • subprocess-fd-problem.py: Code triggering the problem
    		•

    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 = 'https://github.com/gpshead'
closed_at = <Date 2008-06-01.23:45:30.038>
created_at = <Date 2008-05-08.12:22:43.777>
labels = ['library', 'performance']
title = 'subprocess.py leaks fd in communicate'
updated_at = <Date 2008-06-01.23:45:29.996>
user = 'https://github.com/jrosdahl'

    bugs.python.org fields:

    activity = <Date 2008-06-01.23:45:29.996>
actor = 'gregory.p.smith'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2008-06-01.23:45:30.038>
closer = 'gregory.p.smith'
components = ['Library (Lib)']
creation = <Date 2008-05-08.12:22:43.777>
creator = 'jrosdahl'
dependencies = []
files = ['10221']
hgrepos = []
issue_num = 2791
keywords = ['patch']
message_count = 4.0
messages = ['66415', '67068', '67391', '67618']
nosy_count = 3.0
nosy_names = ['jrosdahl', 'gregory.p.smith', 'zanella']
pr_nums = []
priority = 'low'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue2791'
versions = ['Python 2.6', 'Python 2.5']
    @jrosdahl
    Copy link
    Mannequin Author

    jrosdahl mannequin commented May 8, 2008

    The optimization in SVN rev 38556 seems to have changed
    Popen.communicate's behavior when stdout is subprocess.PIPE (and maybe
    for other cases as well).

    See the attached file. In Python 2.4.5, all three counts are the same.
    In Python 2.5.2, the middle count has increased by 1. In other words: A
    file descriptor is leaked until the last reference to the Popen instance
    is dropped.

    @jrosdahl jrosdahl mannequin added stdlib Python modules in the Lib dir performance Performance or resource usage labels May 8, 2008
    @zanella
    Copy link
    Mannequin

    zanella mannequin commented May 19, 2008

    I don't know a lot about the matter at hand, that's why I'm not gonna
    append a patch.

    On "_communicate()" after a pipe is read it's closed, doing the same on
    "communicate()" seems to solve the issue of the extra pipe:

    """
    if [self.stdin, self.stdout, self.stderr].count(None) >= 2:
    stdout = None
    stderr = None
    if self.stdin:
    if input:
    self.stdin.write(input)
    self.stdin.close()
    elif self.stdout:
    stdout = self.stdout.read()
    + self.stdout.close()
    elif self.stderr:
    stderr = self.stderr.read()
    + self.stderr.close()
    self.wait()
    return (stdout, stderr)

    """

    Tested on "Python 2.6a2+ (trunk:62767M, May 19 2008, 13:11:07)".

    @gpshead gpshead self-assigned this May 26, 2008
    @gpshead
    Copy link
    Member

    gpshead commented May 26, 2008

    Thanks zanella. Fixed in 2.6 trunk r63724. I created a unit test based
    on jrosdahl's example code.

    I will backport it to release25-maint after it has baked in trunk for a bit.

    @gpshead
    Copy link
    Member

    gpshead commented Jun 1, 2008

    fixed in release25-maint r63881.

    @gpshead gpshead closed this as completed Jun 1, 2008
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @gpshead gpshead

    Labels
    performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @gpshead