[ruby-core:105463] [Ruby master Feature#18228] Add a `timeout` option to `IO.copy_stream`

["Eregon (Benoit Daloze)" <noreply@ruby-lang.org>]

Issue #18228 has been updated by Eregon (Benoit Daloze).


byroot (Jean Boussier) wrote in #note-4:
> I think it can? But from my initial research I was under the impression that using ALARM to interrupt system calls was a bit frowned upon and that `poll/select` was generally regarded as preferable.

It's what Ruby already uses to interrupt blocking system calls for Ruby interrupts like signals, Thread#raise, etc (as you might already know), so I think it's fine.
Making an IO non-blocking for IO.copy_stream might have side effects, but maybe they are fine.

----------------------------------------
Feature #18228: Add a `timeout` option to `IO.copy_stream`
https://bugs.ruby-lang.org/issues/18228#change-93913

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
----------------------------------------
### 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

- It is [used by `webrick`](https://github.com/ruby/webrick/commit/54be684da9d993ad6c237e2e9853eb98bcbaae6e).
- `Net::HTTP` uses it to send request body if they are IOs, but [it is used with a "fake IO" to allow for timeouts](https://github.com/ruby/net-http/pull/27), so `sendfile(2)` &co are never used.
- [A proof of concept of integrating in puma shows a 2x speedup](https://github.com/puma/puma/pull/2703). 
- [Various other HTTP client could use it as well](https://github.com/nahi/httpclient/pull/383).
- I used it in private projects to download and upload large archives in and out of Google Cloud Storage with great effects.

### 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)`](https://man7.org/linux/man-pages/man2/copy_file_range.2.html) (linux) and [`fcopyfile(2)`](https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/fcopyfile.3.html) (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.






-- 
https://bugs.ruby-lang.org/