Project

General

Profile

Actions

Feature #18228

open

Add a `timeout` option to `IO.copy_stream`

Added by byroot (Jean Boussier) 9 months ago. Updated 9 months ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:105450]

Description

Context

In many situations dealing with large files, IO.copy_stream when usable bring major performance gains (often twice faster at the very least). And more importantly, when the copying is deferred to the kernel, the performance is much more consistent as it is less impacted by the CPU utilization on the machine.

However, it is often unsafe to use because it doesn't have a timeout, so you can only use it if both the source and destination IOs are trusted, otherwise it is trivial for an attacker to DOS the service by reading the response very slowly.

Some examples

Possible implementation

The main difficulty is that the underlying sycalls don't have a timeout either.

The main syscall used in these scenarios is sendfile(2). It doesn't have a timeout parameter, however if called on file descriptors with O_NONBLOCK it does return early and allow for a select/poll loop. I did a very quick and dirty experiment with this, and it does seem to work.

The other two accelerating syscalls are copy_file_range(2) (linux) and fcopyfile(2) (macOS). Neither have a timeout, and neither manpage document an EAGAIN / EWOULDBLOCK error. However these syscalls are limited to real file copies, generally speaking timeouts for real files are less of a critical need, so it would be possible to simply not use these syscalls if a timeout is provided.

Interface

copy_stream(src, dst, copy_length, src_offset, timeout)
or copy_stream(src, dst, copy_length, src_offset, timeout: nil)

As for the return value in case of a timeout, it is important to convey both that a timeout happened, and the number of bytes that were copied, otherwise it makes retries impossible.

  • It could simply returns the number of byte, and let the caller compare it to the expected number of bytes copied, but that wouldn't work in cases where the size of src isn't known.
  • It could return -1 - bytes_copied, not particularly elegant but would work.
  • It could return multiple values or some kind of result object when a timeout is provided.
  • It could raise an error, with bytes_copied as an attribute on the error.

Or alternatively copy_stream would be left without a timeout, and some kind of copy_stream2 would be introduced so that copy_stream return value wouldn't be made inconsistent.


Related issues 1 (0 open1 closed)

Related to Ruby master - Bug #17933: `Net::HTTP#write_timeout` doesn't work with `body_stream`ClosedActions
Actions

Also available in: Atom PDF