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.

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

codecs.open leaks file descriptor when invalid encoding is passed #83152

Closed
BrockMendel mannequin opened this issue Dec 4, 2019 · 12 comments
Closed

codecs.open leaks file descriptor when invalid encoding is passed #83152

BrockMendel mannequin opened this issue Dec 4, 2019 · 12 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes easy performance Performance or resource usage stdlib Python modules in the Lib dir topic-IO

Comments

@BrockMendel
Copy link
Mannequin

BrockMendel mannequin commented Dec 4, 2019

BPO 38971
Nosy @serhiy-storchaka, @MojoVampire, @miss-islington, @tirkarthi, @caporta
PRs
  • bpo-38971: open file in codecs.open() closes if exception raised #17666
  • [3.7] bpo-38971: Open file in codecs.open() closes if exception raised. (GH-17666) #18733
  • [3.8] bpo-38971: Open file in codecs.open() closes if exception raised. (GH-17666) #18734
    		•

    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-03-02.08:16:43.886>
created_at = <Date 2019-12-04.16:51:47.592>
labels = ['easy', '3.8', '3.9', 'expert-IO', 'performance', '3.7', 'library']
title = 'codecs.open leaks file descriptor when invalid encoding is passed'
updated_at = <Date 2020-03-02.08:16:43.885>
user = 'https://bugs.python.org/BrockMendel'

    bugs.python.org fields:

    activity = <Date 2020-03-02.08:16:43.885>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2020-03-02.08:16:43.886>
closer = 'serhiy.storchaka'
components = ['Library (Lib)', 'IO']
creation = <Date 2019-12-04.16:51:47.592>
creator = 'Brock Mendel'
dependencies = []
files = []
hgrepos = []
issue_num = 38971
keywords = ['patch', 'easy']
message_count = 12.0
messages = ['357811', '357812', '357814', '357815', '357818', '357834', '357838', '360731', '363128', '363136', '363137', '363138']
nosy_count = 6.0
nosy_names = ['serhiy.storchaka', 'josh.r', 'miss-islington', 'xtreak', 'Brock Mendel', 'caporta']
pr_nums = ['17666', '18733', '18734']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue38971'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']
    @BrockMendel
    Copy link
    Mannequin Author

    BrockMendel mannequin commented Dec 4, 2019

    xref pandas-dev/pandas#30034

    codecs.open does file = open(...) before validating the encoding kwarg, leaving the open file behind if that validation raises.

    @tirkarthi
    Copy link
    Member

    Does using with block similar to https://bugs.python.org/issue22831 solve this problem?

    @BrockMendel
    Copy link
    Mannequin Author

    BrockMendel mannequin commented Dec 4, 2019

    Does using with block similar to https://bugs.python.org/issue22831 solve this problem?

    The motivating use case uses `with codecs.open(buf, "w", encoding=encoding) as f:`

    https://github.com/pandas-dev/pandas/blob/master/pandas/io/formats/format.py#L498

    @tirkarthi
    Copy link
    Member

    Ah okay, thanks for the detail. Forgot there should be an open file handle to be returned by codecs.open

    @serhiy-storchaka
    Copy link
    Member

    Add

    try:
    ...
except:
    file.close()
    raise

    @serhiy-storchaka serhiy-storchaka added easy stdlib Python modules in the Lib dir topic-IO 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes performance Performance or resource usage labels Dec 4, 2019
    @MojoVampire
    Copy link
    Mannequin

    MojoVampire mannequin commented Dec 4, 2019

    Any reason not to just defer opening the file until after the codec has been validated, so the resource acquisition comes last?

    @serhiy-storchaka
    Copy link
    Member

    Many reasons.

    1. It is simpler.
    2. We will need a try/except in any case to prevent a leak if an exception be raised by other code following open().
    3. It matches the behavior of io.open().

    @caporta
    Copy link
    Mannequin

    caporta mannequin commented Jan 26, 2020

    Just quickly pinging the thread as a friendly reminder that PR 17666 is open and potentially close to mergeable, as it's been through two review cycles already (thanks Serhiy). If someone has the bandwidth to take another look, it would be greatly appreciated. Thanks!

    @serhiy-storchaka
    Copy link
    Member

    New changeset 2565ede by Chris A in branch 'master':
    bpo-38971: Open file in codecs.open() closes if exception raised. (GH-17666)
    2565ede

    @miss-islington
    Copy link
    Contributor

    New changeset f4d709f by Miss Islington (bot) in branch '3.7':
    bpo-38971: Open file in codecs.open() closes if exception raised. (GH-17666)
    f4d709f

    @miss-islington
    Copy link
    Contributor

    New changeset f28b0c7 by Miss Islington (bot) in branch '3.8':
    bpo-38971: Open file in codecs.open() closes if exception raised. (GH-17666)
    f28b0c7

    @serhiy-storchaka
    Copy link
    Member

    It is too later for 2.7.

    @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
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes easy performance Performance or resource usage stdlib Python modules in the Lib dir topic-IO
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @serhiy-storchaka @tirkarthi @miss-islington