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

SystemError if custom opener returns -1 #71253

Closed
warsaw opened this issue May 19, 2016 · 8 comments
Closed

SystemError if custom opener returns -1 #71253

warsaw opened this issue May 19, 2016 · 8 comments
Assignees
Labels
topic-IO type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@warsaw
Copy link
Member

warsaw commented May 19, 2016

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']

    @warsaw
    Copy link
    Member Author

    warsaw commented May 19, 2016

    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'

    @warsaw warsaw added topic-IO type-crash A hard crash of the interpreter, possibly with a core dump labels May 19, 2016
    @pppery
    Copy link
    Mannequin

    pppery mannequin commented May 20, 2016

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

    @warsaw
    Copy link
    Member Author

    warsaw commented May 20, 2016

    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
    Copy link
    Member Author

    warsaw commented May 20, 2016

    Here's a proposed fix.

    @warsaw warsaw self-assigned this May 20, 2016
    @serhiy-storchaka
    Copy link
    Member

    Added comments on Rietveld.

    @serhiy-storchaka
    Copy link
    Member

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

    @warsaw
    Copy link
    Member Author

    warsaw commented May 23, 2016

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

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 8, 2016

    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

    @warsaw warsaw closed this as completed Jun 8, 2016
    @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
    Labels
    topic-IO type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants