SystemError if custom opener returns -1

[warsaw]

BPO	27066
Nosy	@warsaw, @serhiy-storchaka, @pppery
Files	27066-1.patch 27066-2.patch

^{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/warsaw'
closed_at = <Date 2016-06-08.21:58:54.346>
created_at = <Date 2016-05-19.23:50:49.192>
labels = ['expert-IO', 'type-crash']
title = 'SystemError if custom opener returns -1'
updated_at = <Date 2016-06-09.01:42:01.244>
user = 'https://github.com/warsaw'

bugs.python.org fields:

activity = <Date 2016-06-09.01:42:01.244>
actor = 'berker.peksag'
assignee = 'barry'
closed = True
closed_date = <Date 2016-06-08.21:58:54.346>
closer = 'barry'
components = ['IO']
creation = <Date 2016-05-19.23:50:49.192>
creator = 'barry'
dependencies = []
files = ['42907', '42956']
hgrepos = []
issue_num = 27066
keywords = ['patch']
message_count = 8.0
messages = ['265901', '265902', '265903', '265905', '265908', '266176', '266194', '267904']
nosy_count = 4.0
nosy_names = ['barry', 'python-dev', 'serhiy.storchaka', 'ppperry']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue27066'
versions = ['Python 3.5', 'Python 3.6']

Linked PRs

• gh-71253: Match _io exception in _pyio #133985
• gh-71253: Run unclosed file test on all io implementations #134164
• [3.14] gh-71253: Match _io exception in _pyio (gh-133985) #134430
• [3.13] gh-71253: Match _io exception in _pyio (gh-133985) #134431

[warsaw]

Let's say you use a custom opener, and that opener happens to return exactly -1. You end up with a SystemError because NULL got returned without an exception being set:

def negative(fname, flags):
    return -1


with open('/tmp/foo.txt', 'w', encoding='utf-8', opener=negative) as fp:
    print('oops', file=fp)

% python3 /tmp/foo.py 
Traceback (most recent call last):
  File "/tmp/foo.py", line 5, in <module>
    with open('/tmp/foo.txt', 'w', encoding='utf-8', opener=negative) as fp:
SystemError: <class '_io.FileIO'> returned NULL without setting an error

Anything else and you get a relatively decent exception. E.g. return -2 and you get an OSError. Raise an exception and you get that exception.

The problem is pretty clear to see; when an opener is set, after coercing the fd to an integer, the check is made for that integer being -1, and then it jumps right to the exit.

Let's say you return some non-integer, like 'foo'. Then the _PyLong_AsInt() will fail and a proper exception will be set. So I think the "if (self->fd == -1)" clause just needs to check for an exception set first and set one if there isn't one before it does the "goto error". I guess you'd want to see the same exception as if it returned say, -2:

Traceback (most recent call last):
  File "/tmp/foo.py", line 5, in <module>
    with open('/tmp/foo.txt', 'w', encoding='utf-8', opener=negative) as fp:
OSError: [Errno 0] Error: '/tmp/foo.txt'

[pppery]

Also, OSError [Errno 0] Error isn't the most helpful error message.

[warsaw]

On May 20, 2016, at 12:12 AM, ppperry wrote:

Also, OSError [Errno 0] Error isn't the most helpful error message.

No, definitely not. ;)

[warsaw]

Here's a proposed fix.

[serhiy-storchaka]

Added comments on Rietveld.

[serhiy-storchaka]

In general LGTM, but I left style comment on Rietveld. And you can use assertRaisesRegex in tests if you prefers.

[warsaw]

BTW, I may wait to commit this until after we've moved to GitHub. :)

[python-dev]

New changeset 4af64ab34eef by Barry Warsaw in branch '3.5':
Issue bpo-27066: Fixed SystemError if a custom opener (for open()) returns
https://hg.python.org/cpython/rev/4af64ab34eef

New changeset 84c91d7d4667 by Barry Warsaw in branch 'default':
Issue bpo-27066: Fixed SystemError if a custom opener (for open()) returns a
https://hg.python.org/cpython/rev/84c91d7d4667