Ruby

I submitted a pull request to solve this issue: https://github.com/ruby/ruby/pull/1575

jean.boussier@gmail.com wrote:

Issue #13396 has been updated by byroot (Jean Boussier).

I submitted a pull request to solve this issue: https://github.com/ruby/ruby/pull/1575

Thanks, I think having a write timeout for Net::HTTP is long overdue.

I'm not approved to make public API changes but would like it accepted.

I would also like to add native timeout support to IO.copy_stream
(or improve stdlib timeout support for safety by implementing
natively).

Note: I checked your commit c789ab8df5c8d6e0643ccd481f748014f0066345
by having a "fetch = +refs/pull/:refs/remotes/ruby/pull/"
line in a "remote" section of my .git/config. I did not
use any proprietary API or JavaScript to view your changes.

I tried setting setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDTIMEO, ...) on the client socket, but without success. However adding a Timeout.timeout call around req.exec did work.

Under Linux, SO_SNDTIMEO won't work if the the socket was set to
nonblocking which would cause Ruby to wait with ppoll (select on
other platforms).

I would also like to add native timeout support to IO.copy_stream

That would be the best indeed.

Only supports Linux:

@@ -10372,11 +10383,23 @@ static int nogvl_wait_for_single_fd(int fd, short events) { struct pollfd fds; + struct timespec ts, *tsp = NULL;   fds.fd = fd; fds.events = events;  - return poll(&fds, 1, -1); + if (is_socket(fd, Qnil)) { + struct timeval timeout; + socklen_t sz = sizeof(timeout); + if (getsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeout, &sz) == 0 && + timeout.tv_sec != 0 || timeout.tv_usec != 0) { + ts.tv_sec = timeout.tv_sec; + ts.tv_nsec = timeout.tv_usec * 1000; + tsp = &ts; + } + } + + return ppoll(&fds, 1, tsp, NULL);  }   static int @@ -10445,6 +10468,9 @@ nogvl_copy_stream_wait_write(struct copy_stream_struct *stp) #endif } while (ret == -1 && maygvl_copy_stream_continue_p(0, stp));  + if (ret == 0) { + return -1; + }  if (ret == -1) { stp->syserr = IOWAIT_SYSCALL; stp->error_no = errno; 

naruse@airemix.jp wrote:

Only supports Linux:

...And only with sockets where O_NONBLOCK is not set, because
the O_NONBLOCK flag will ignore SO_*TIMEO.

Anyways, I don't think using SO_*TIMEO for Ruby is worth it.

I think Ruby can gain an internal timeout mechanism which can
hook into all rb_wait APIs and raise Timeout::TimeoutError as
appropriate.

I will consider that for my work-in-progress on auto-fibers;
all APIs will have a timeout arg: [ruby-core:81244]

I just noticed that just use write_nonblock can solve this ticket:

diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb index 7ec636b384..2a806caeb6 100644 --- a/lib/net/protocol.rb +++ b/lib/net/protocol.rb @@ -77,6 +77,12 @@ class OpenTimeout < Timeout::Error; end class ReadTimeout < Timeout::Error; end + ## + # WriteTimeout, a subclass of Timeout::Error, is raised if a chunk of the + # response cannot be read within the read_timeout. + + class WriteTimeout < Timeout::Error; end +  class BufferedIO #:nodoc: internal use only def initialize(io, read_timeout: 60, continue_timeout: nil, debug_output: nil) @@ -237,9 +243,32 @@ def writing def write0(*strs) @debug_output << strs.map(&:dump).join if @debug_output - len = @io.write(*strs) - @written_bytes += len - len + case len = @io.write_nonblock(*strs, exception: false) + when Integer + orig_len = len + strs.each_with_index do |str, i| + len -= str.bytesize + if len == 0 + if strs.size == i+1 + @written_bytes += orig_len + return orig_len + else + strs = strs[i+1..] # rest + break + end + elsif len < 0 + strs = strs[i..] # str and rest + strs[0] = str[len, -len] + break + else # len > 0 + # next + end + end + # continue looping + when :wait_writable + @io.to_io.wait_writable(@write_timeout) or raise Net::ReadTimeout + # continue looping + end while true  end # 

naruse@ruby-lang.org wrote:

https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63587

For OpenSSL, I think you need to expect : wait_readable on
write_nonblock, too.

Anyways, the code for handling partial write_nonblock case is verbose.
One day, I would like to:

1. integrate Timeout into core
2. make all SOCK_STREAM sockets non-blocking by default
3. Make rb_wait_for_single_fd aware of Timeouts

So we can use:

Timeout.timeout(@write_timeout) { @io.write(strs) }

And no new background threads get spawned.

P.S.: If Ruby were LGPL-2.1+, I would steal the ccan/timer module which
is optimized for frequently-expiring timers and be done with 1),
already.

Eric Wong normalperson@yhbt.net wrote:

Anyways, the code for handling partial write_nonblock case is verbose.
One day, I would like to:

1. integrate Timeout into core
2. make all SOCK_STREAM sockets non-blocking by default
3. Make rb_wait_for_single_fd aware of Timeouts

FYI, I'm close to having a patch ready for 1) and 3);
but maybe 3 is optional, even.

So we can use:

Timeout.timeout(@write_timeout) { @io.write(strs) } 

And no new background threads get spawned.

P.S.: If Ruby were LGPL-2.1+, I would steal the ccan/timer module which
is optimized for frequently-expiring timers and be done with 1),
already.

ccan/timer may not be the right tool for the job (more on this
later).

There's an open issue on Net::HTTP#write_timeout not working when body_stream is used: https://bugs.ruby-lang.org/issues/17933

I'm just sharing this here for visibility.