classification
Title: os.sendfile can return EINVAL on Solaris
Type: crash Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: giampaolo.rodola Nosy List: giampaolo.rodola, kulikjak
Priority: normal Keywords: patch

Created on 2019-04-12 08:56 by kulikjak, last changed 2019-05-30 06:06 by giampaolo.rodola. This issue is now closed.

Files
File name Uploaded Description Edit
sendfile.patch kulikjak, 2019-04-12 08:56 Patch of sendfile in socket.py
example.py kulikjak, 2019-04-15 08:28 issue reproducing script
Pull Requests
URL Status Linked Edit
PR 13675 merged giampaolo.rodola, 2019-05-30 04:03
Messages (5)
msg340017 - (view) Author: Jakub Kulik (kulikjak) * Date: 2019-04-12 08:56
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
msg340063 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2019-04-12 15:30
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.
msg340241 - (view) Author: Jakub Kulik (kulikjak) * Date: 2019-04-15 08:28
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.
msg343948 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2019-05-30 04:23
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.
msg343950 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2019-05-30 06:05
New changeset 413d955f8ec88a7183f91d7ad8b0ff7def803de3 by Giampaolo Rodola in branch 'master':
bpo-36610: shutil.copyfile(): use sendfile() on Linux only (GH-13675)
https://github.com/python/cpython/commit/413d955f8ec88a7183f91d7ad8b0ff7def803de3
History
Date User Action Args
2019-05-30 06:06:39giampaolo.rodolasetstatus: open -> closed
assignee: giampaolo.rodola
resolution: fixed
stage: patch review -> resolved
2019-05-30 06:05:52giampaolo.rodolasetmessages: + msg343950
2019-05-30 04:23:35giampaolo.rodolasetmessages: + msg343948
versions: - Python 3.7
2019-05-30 04:03:29giampaolo.rodolasetstage: patch review
pull_requests: + pull_request13563
2019-04-15 08:28:13kulikjaksetfiles: + example.py

messages: + msg340241
2019-04-12 15:30:31giampaolo.rodolasetnosy: + giampaolo.rodola
messages: + msg340063
2019-04-12 08:56:34kulikjakcreate