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

os.sendfile can return EINVAL on Solaris #80791

Closed
kulikjak mannequin opened this issue Apr 12, 2019 · 5 comments
Closed

os.sendfile can return EINVAL on Solaris #80791

kulikjak mannequin opened this issue Apr 12, 2019 · 5 comments
Assignees
Labels
3.8 only security fixes stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@kulikjak
Copy link
Mannequin

kulikjak mannequin commented Apr 12, 2019

BPO 36610
Nosy @giampaolo, @kulikjak
PRs
  • bpo-36610: shutil.copyfile(): use sendfile() on Linux only #13675
  • Files
  • sendfile.patch: Patch of sendfile in socket.py
  • example.py: issue reproducing script
  • 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/giampaolo'
    closed_at = <Date 2019-05-30.06:06:39.418>
    created_at = <Date 2019-04-12.08:56:34.032>
    labels = ['3.8', 'library', 'type-crash']
    title = 'os.sendfile can return EINVAL on Solaris'
    updated_at = <Date 2019-05-30.06:06:39.418>
    user = 'https://github.com/kulikjak'

    bugs.python.org fields:

    activity = <Date 2019-05-30.06:06:39.418>
    actor = 'giampaolo.rodola'
    assignee = 'giampaolo.rodola'
    closed = True
    closed_date = <Date 2019-05-30.06:06:39.418>
    closer = 'giampaolo.rodola'
    components = ['Library (Lib)']
    creation = <Date 2019-04-12.08:56:34.032>
    creator = 'kulikjak'
    dependencies = []
    files = ['48262', '48266']
    hgrepos = []
    issue_num = 36610
    keywords = ['patch']
    message_count = 5.0
    messages = ['340017', '340063', '340241', '343948', '343950']
    nosy_count = 2.0
    nosy_names = ['giampaolo.rodola', 'kulikjak']
    pr_nums = ['13675']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue36610'
    versions = ['Python 3.8']

    @kulikjak
    Copy link
    Mannequin Author

    kulikjak mannequin commented Apr 12, 2019

    Hi,

    We have several tests failing on Solaris due to the slightly different behavior of os.sendfile function. Sendfile on Solaris can raise EINVAL if offset is equal or bigger than the size of the file (Python expects that it will return 0 bytes sent in that case).

    I managed to patch socked.py with additional checks on two places (patch attached), Python 3.8 introduced sendfile in shutil.py module, where I don't have fsize variable so easily accessible and so I am unsure what to do with it. Also, I am not even sure if this is a correct way to handle this. Maybe this should be patched somewhere in the .c file? Or there might be other systems with the same behavior and all I need to do is adjust some define guards there...

    EINVAL can also mean other things and so I guess I cannot just catch that errno and continue as with returned 0.

    Thanks

    @kulikjak kulikjak mannequin added 3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump labels Apr 12, 2019
    @giampaolo
    Copy link
    Contributor

    Can you paste the traceback or are you able to reproduce the bug via a script? sendfile implementation is supposed to giveup if no data was sent on first call, so I suppose this happen later? If for any reason it turns out sendfile() is broken on Solaris or behave differently than on Linux I think I prefer to be safe than sorry and support it on Linux only.

    @kulikjak
    Copy link
    Mannequin Author

    kulikjak mannequin commented Apr 15, 2019

    Here is a traceback from one failed test:

    test test_lib2to3 failed
    ======================================================================
    ERROR: test_refactor_file_write_unchanged_file (lib2to3.tests.test_refactor.TestRefactoringTool)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/tmp/Python-3.8.0a3/Lib/lib2to3/tests/test_refactor.py", line 228, in test_refactor_file_write_unchanged_file
        self.check_file_refactoring(test_file, fixers=(),
      File "/tmp/Python-3.8.0a3/Lib/lib2to3/tests/test_refactor.py", line 183, in check_file_refactoring
        test_file = self.init_test_file(test_file)
      File "/tmp/Python-3.8.0a3/Lib/lib2to3/tests/test_refactor.py", line 202, in init_test_file
        shutil.copy(test_file, tmpdir)
      File "/tmp/Python-3.8.0a3/Lib/shutil.py", line 401, in copy
        copyfile(src, dst, follow_symlinks=follow_symlinks)
      File "/tmp/Python-3.8.0a3/Lib/shutil.py", line 266, in copyfile
        _fastcopy_sendfile(fsrc, fdst)
      File "/tmp/Python-3.8.0a3/Lib/shutil.py", line 165, in _fastcopy_sendfile
        raise err
      File "/tmp/Python-3.8.0a3/Lib/shutil.py", line 145, in _fastcopy_sendfile
        sent = os.sendfile(outfd, infd, offset, blocksize)
    OSError: [Errno 22] Invalid argument: '/tmp/Python-3.8.0a3/Lib/lib2to3/tests/data/fixers/parrot_example.py' -> '/tmp/2to3-test_refactoruxve6nrz/parrot_example.py'

    I also have a script which can reproduce this (attached). It happens to me every time my offset is bigger or equal than the size of the file, no matter whether the file is empty or offset is bigger than the size right from the start, even when in a loop, sending in several parts. The attached example works on my Linux machine (outputs 12 and 0), but breaks on Solaris (12 and exception).

    As I have said before, it might be just that Python (specifically sendfile implementation) is not compiled correctly on Solaris and changes to define guards might fix that.

    @giampaolo
    Copy link
    Contributor

    I currently have no Solaris box to test this against and am currently short on time but I'm of the opinion that because of:

    > Sendfile on Solaris can raise EINVAL if offset is equal or bigger than the size
    > of the file (Python expects that it will return 0 bytes sent in that case).

    ...and the fact that beta1 will happen soon it's safer to use sendfile() on Linux only. Googling for "solaris + sendfile" also raises suspicions that sendfile() may be broken on Solaris. shutil.copyfile() is too central to risk breaking it.

    We may also want to look at socket.sendfile() implementation and add tests for large files for both functions (socket.sendfile() and shutil.copyfile()). I'm adding that to my TODO list.

    @giampaolo giampaolo removed the 3.7 (EOL) end of life label May 30, 2019
    @giampaolo
    Copy link
    Contributor

    New changeset 413d955 by Giampaolo Rodola in branch 'master':
    bpo-36610: shutil.copyfile(): use sendfile() on Linux only (GH-13675)
    413d955

    @giampaolo giampaolo self-assigned this May 30, 2019
    @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
    3.8 only security fixes stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant