Limit max sendfile chunk to 0x7ffff000

[asvetlov]

BPO	35179
Nosy	@vstinner, @giampaolo, @asvetlov, @1st1

^{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 = None
closed_at = <Date 2018-11-06.22:31:55.302>
created_at = <Date 2018-11-06.16:41:31.211>
labels = ['3.8', '3.7', 'library', 'invalid', 'expert-asyncio']
title = 'Limit max sendfile chunk to 0x7ffff000'
updated_at = <Date 2018-11-06.22:37:23.040>
user = 'https://github.com/asvetlov'

bugs.python.org fields:

activity = <Date 2018-11-06.22:37:23.040>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-11-06.22:31:55.302>
closer = 'asvetlov'
components = ['Library (Lib)', 'asyncio']
creation = <Date 2018-11-06.16:41:31.211>
creator = 'asvetlov'
dependencies = []
files = []
hgrepos = []
issue_num = 35179
keywords = []
message_count = 7.0
messages = ['329366', '329386', '329387', '329388', '329390', '329393', '329394']
nosy_count = 4.0
nosy_names = ['vstinner', 'giampaolo.rodola', 'asvetlov', 'yselivanov']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue35179'
versions = ['Python 3.7', 'Python 3.8']

[asvetlov]

On Linux maximum data size for sendfile call is 0x7ffff000:

sendfile() will transfer at most 0x7ffff000 (2,147,479,552) bytes, returning the number of bytes actually
transferred. (This is true on both 32-bit and 64-bit systems.)

Limiting max block size to this value on all OSes makes sense: splitting transferring the very huge file into several syscalls doesn't hurt performance anyway.

Windows uses DWORD for size in TransmitFile, so the size is limited as well.

[giampaolo]

Do you mean raising an exception if "count" argument is passed and > 2,147,479,552? In that case I think asyncio's sendfile() should simply do the math to transmit that many bytes by taking into account that os.sendfile() may return less bytes than requested. With non-blocking sockets in particular that is true regardless from the size being passed.

[vstinner]

os.write(data) can write less than len(data) bytes. It's by contract, as socket.send(data) can send less than len(data).

It is used to truncated the length argument to INT_MAX, to be able to cast it to an int on Windows. Extract of _Py_write():

#ifdef MS_WINDOWS
    if (count > 32767 && isatty(fd)) {
        /* Issue python/cpython#55604: the Windows console returns an error (12: not
           enough space error) on writing into stdout if stdout mode is
           binary and the length is greater than 66,000 bytes (or less,
           depending on heap usage). */
        count = 32767;
    }
#endif
    if (count > _PY_WRITE_MAX) {
        count = _PY_WRITE_MAX;
    }

with:

#if defined(MS_WINDOWS) || defined(__APPLE__)
    /* On Windows, the count parameter of read() is an int (bpo-9015, bpo-9611).
       On macOS 10.13, read() and write() with more than INT_MAX bytes
       fail with EINVAL (bpo-24658). */
#   define _PY_READ_MAX  INT_MAX
#   define _PY_WRITE_MAX INT_MAX
#else
    /* write() should truncate the input to PY_SSIZE_T_MAX bytes,
       but it's safer to do it ourself to have a portable behaviour */
#   define _PY_READ_MAX  PY_SSIZE_T_MAX
#   define _PY_WRITE_MAX PY_SSIZE_T_MAX
#endif

Can we do something similar for sendfile()?

[vstinner]

In that case I think asyncio's sendfile() should simply do the math to transmit that many bytes by taking into account that os.sendfile() may return less bytes than requested.

The internal sendfile() implementation in asyncio already loops until all bytes are sent. Extract of unix_events.py:

    def _sock_sendfile_native_impl(self, fut, registered_fd, sock, fileno,
                                   offset, count, blocksize, total_sent):
        ...
        try:
            sent = os.sendfile(fd, fileno, offset, blocksize)
        except (BlockingIOError, InterruptedError):
            ...
        else:
            if sent == 0:
                # EOF
                self._sock_sendfile_update_filepos(fileno, offset, total_sent)
                fut.set_result(total_sent)
            else:
                offset += sent
                total_sent += sent
                self.add_writer(fd, self._sock_sendfile_native_impl, fut, ...)

asyncio doesn't need to be modified.

[vstinner]

The manual page says:

   sendfile() will transfer  at  most  0x7ffff000  (2,147,479,552)  bytes,
   returning  the  number of bytes actually transferred.  (This is true on
   both 32-bit and 64-bit systems.)

I understand that you can pass a larger length, but a single sendfile() function call will never copy more than 0x7ffff000 bytes. I see nothing wrong here.

If you want to be sure, try to copy more bytes than 0x7ffff000 and see what happens :-) Use strace to check system calls.

[asvetlov]

My initial thought was that os.sendfile() raises OSError on sending more than 0x7fff_f000 but I was wrong.
I've checked it, everything works as Victor described.

Thank you guys for the feedback.

[vstinner]

I've checked it, everything works as Victor described.

Yeah! That's great when it just works!