classification
Title: codecs.open leaks file descriptor when invalid encoding is passed
Type: resource usage Stage: resolved
Components: IO, Library (Lib) Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Brock Mendel, caporta, josh.r, miss-islington, serhiy.storchaka, xtreak
Priority: normal Keywords: easy, patch

Created on 2019-12-04 16:51 by Brock Mendel, last changed 2020-03-02 08:16 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17666 merged python-dev, 2019-12-19 22:31
PR 18733 merged miss-islington, 2020-03-02 07:42
PR 18734 merged miss-islington, 2020-03-02 07:43
Messages (12)
msg357811 - (view) Author: Brock Mendel (Brock Mendel) Date: 2019-12-04 16:51
xref https://github.com/pandas-dev/pandas/pull/30034

codecs.open does `file = open(...)` before validating the encoding kwarg, leaving the open file behind if that validation raises.
msg357812 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-12-04 17:01
Does using with block similar to https://bugs.python.org/issue22831 solve this problem?
msg357814 - (view) Author: Brock Mendel (Brock Mendel) Date: 2019-12-04 17:19
> 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
msg357815 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-12-04 18:01
Ah okay, thanks for the detail. Forgot there should be an open file handle to be returned by codecs.open
msg357818 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-12-04 19:34
Add

    try:
        ...
    except:
        file.close()
        raise
msg357834 - (view) Author: Josh Rosenberg (josh.r) * (Python triager) Date: 2019-12-04 22:35
Any reason not to just defer opening the file until after the codec has been validated, so the resource acquisition comes last?
msg357838 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-12-05 05:26
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().
msg360731 - (view) Author: Chris Aporta (caporta) * Date: 2020-01-26 18:33
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!
msg363128 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-03-02 06:39
New changeset 2565edec2c974b2acca03b4cc5025e83f903ddd7 by Chris A in branch 'master':
bpo-38971: Open file in codecs.open() closes if exception raised. (GH-17666)
https://github.com/python/cpython/commit/2565edec2c974b2acca03b4cc5025e83f903ddd7
msg363136 - (view) Author: miss-islington (miss-islington) Date: 2020-03-02 08:02
New changeset f4d709f4a3c69bd940bd6968a70241277132bed7 by Miss Islington (bot) in branch '3.7':
bpo-38971: Open file in codecs.open() closes if exception raised. (GH-17666)
https://github.com/python/cpython/commit/f4d709f4a3c69bd940bd6968a70241277132bed7
msg363137 - (view) Author: miss-islington (miss-islington) Date: 2020-03-02 08:03
New changeset f28b0c74e54a133cb0287b4297af67d0d7c14d6e by Miss Islington (bot) in branch '3.8':
bpo-38971: Open file in codecs.open() closes if exception raised. (GH-17666)
https://github.com/python/cpython/commit/f28b0c74e54a133cb0287b4297af67d0d7c14d6e
msg363138 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-03-02 08:16
It is too later for 2.7.
History
Date User Action Args
2020-03-02 08:16:43serhiy.storchakasetstatus: open -> closed
versions: - Python 2.7
messages: + msg363138

resolution: fixed
stage: patch review -> resolved
2020-03-02 08:03:11miss-islingtonsetmessages: + msg363137
2020-03-02 08:02:20miss-islingtonsetmessages: + msg363136
2020-03-02 07:43:00miss-islingtonsetpull_requests: + pull_request18091
2020-03-02 07:42:53miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request18090
2020-03-02 06:39:56serhiy.storchakasetmessages: + msg363128
2020-01-26 18:33:16caportasetnosy: + caporta
messages: + msg360731
2019-12-19 22:31:26python-devsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request17132
2019-12-05 05:26:50serhiy.storchakasetmessages: + msg357838
2019-12-04 22:35:40josh.rsetnosy: + josh.r
messages: + msg357834
2019-12-04 19:34:41serhiy.storchakasetversions: + Python 2.7, Python 3.7, Python 3.8, Python 3.9
messages: + msg357818

components: + Library (Lib), IO
keywords: + easy
type: resource usage
stage: needs patch
2019-12-04 18:01:56xtreaksetmessages: + msg357815
2019-12-04 17:19:31Brock Mendelsetmessages: + msg357814
2019-12-04 17:01:30xtreaksetnosy: + serhiy.storchaka
messages: + msg357812
2019-12-04 16:54:51xtreaksetnosy: + xtreak
2019-12-04 16:51:47Brock Mendelcreate