https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112017-04-04T09:44:21ZRuby Issue Tracking SystemRuby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=640572017-04-04T09:44:21Zbyroot (Jean Boussier)byroot@ruby-lang.org
<ul></ul><p>I submitted a pull request to solve this issue: <a href="https://github.com/ruby/ruby/pull/1575" class="external">https://github.com/ruby/ruby/pull/1575</a></p> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=640642017-04-04T18:08:41Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p><a href="mailto:jean.boussier@gmail.com" class="email">jean.boussier@gmail.com</a> wrote:</p>
<blockquote>
<p>Issue <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Net::HTTP has no write timeout (Closed)" href="https://bugs.ruby-lang.org/issues/13396">#13396</a> has been updated by byroot (Jean Boussier).</p>
<p>I submitted a pull request to solve this issue: <a href="https://github.com/ruby/ruby/pull/1575" class="external">https://github.com/ruby/ruby/pull/1575</a></p>
</blockquote>
<p>Thanks, I think having a write timeout for Net::HTTP is long overdue.</p>
<p>I'm not approved to make public API changes but would like it accepted.</p>
<p>I would also like to add native timeout support to IO.copy_stream<br>
(or improve stdlib timeout support for safety by implementing<br>
natively).</p>
<p>Note: I checked your commit c789ab8df5c8d6e0643ccd481f748014f0066345<br>
by having a "fetch = +refs/pull/<em>:refs/remotes/ruby/pull/</em>"<br>
line in a "remote" section of my .git/config. I did not<br>
use any proprietary API or JavaScript to view your changes.</p>
<blockquote>
<p>I tried setting <code>setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDTIMEO, ...)</code> on the client socket, but without success. However adding a <code>Timeout.timeout</code> call around <code>req.exec</code> did work.</p>
</blockquote>
<p>Under Linux, SO_SNDTIMEO won't work if the the socket was set to<br>
nonblocking which would cause Ruby to wait with ppoll (select on<br>
other platforms).</p> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=642022017-04-13T05:42:09Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul><li><strong>Tracker</strong> changed from <i>Bug</i> to <i>Feature</i></li></ul><p>Let me turn it to be a feature request. Will ask matz about it.</p> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=648712017-05-17T10:45:08Zbyroot (Jean Boussier)byroot@ruby-lang.org
<ul></ul><blockquote>
<p>I would also like to add native timeout support to IO.copy_stream</p>
</blockquote>
<p>That would be the best indeed.</p> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=649922017-05-20T18:48:03Znaruse (Yui NARUSE)naruse@airemix.jp
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li><li><strong>Assignee</strong> set to <i>naruse (Yui NARUSE)</i></li></ul><p>The concept Net::HTTP#write_timeout sounds fine.</p>
<blockquote>
<p>However adding a Timeout.timeout call around req.exec did work.</p>
</blockquote>
<p>Timeou.timeout should be avoided because it sometimes caused trouble and now eliminated.<br>
(see also <a href="https://github.com/ruby/ruby/pull/899" class="external">https://github.com/ruby/ruby/pull/899</a>)</p>
<blockquote>
<p>I would also like to add native timeout support to IO.copy_stream</p>
</blockquote>
<p>Agree.</p>
<blockquote>
<p>Note: I checked your commit c789ab8df5c8d6e0643ccd481f748014f0066345</p>
</blockquote>
<p>You may know, <a href="https://github.com/ruby/ruby/pull/1575.patch" class="external">https://github.com/ruby/ruby/pull/1575.patch</a> and <a href="https://github.com/ruby/ruby/pull/1575.diff" class="external">https://github.com/ruby/ruby/pull/1575.diff</a> are also available.<br>
They are written as LINK rel="alternate" in the HTML.</p>
<blockquote>
<blockquote>
<p>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.</p>
</blockquote>
</blockquote>
<blockquote>
<p>Under Linux, SO_SNDTIMEO won't work if the the socket was set to<br>
nonblocking which would cause Ruby to wait with ppoll (select on other platforms).</p>
</blockquote>
<p>Like this?<br>
Maybe though IO#write seems to be kept as is.</p>
<pre><code class="diff syntaxhl" data-language="diff"><span class="p">@@ -1340,6 +1348,17 @@</span> io_binwrite(VALUE str, const char *ptr, long len, rb_io_t *fptr, int nosync)
n -= r;
errno = EAGAIN;
}
<span class="gi">+#ifdef SO_SNDTIMEO
+ if (errno == EAGAIN && is_socket(fptr->fd, fptr->pathv)) {
+ /* Even if EAGAIN, it may be caused by timeout */
+ struct timeval timeout;
+ socklen_t sz = sizeof(timeout);
+ if (getsockopt(fptr->fd, SOL_SOCKET, SO_SNDTIMEO, &timeout, &sz) == 0 &&
+ timeout.tv_sec !=0 || timeout.tv_usec != 0) {
+ return r;
+ }
+ }
+#endif
</span> if (r == -2L)
return -1L;
if (rb_io_wait_writable(fptr->fd)) {
</code></pre> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=650012017-05-20T22:01:07Znaruse (Yui NARUSE)naruse@airemix.jp
<ul></ul><p>Only supports Linux:</p>
<pre><code class="diff syntaxhl" data-language="diff"><span class="p">@@ -10372,11 +10383,23 @@</span> static int
nogvl_wait_for_single_fd(int fd, short events)
{
struct pollfd fds;
<span class="gi">+ struct timespec ts, *tsp = NULL;
</span><span class="err">
</span> fds.fd = fd;
fds.events = events;
<span class="err">
</span><span class="gd">- return poll(&fds, 1, -1);
</span><span class="gi">+ 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);
</span> }
<span class="err">
</span> static int
<span class="p">@@ -10445,6 +10468,9 @@</span> nogvl_copy_stream_wait_write(struct copy_stream_struct *stp)
#endif
} while (ret == -1 && maygvl_copy_stream_continue_p(0, stp));
<span class="err">
</span><span class="gi">+ if (ret == 0) {
+ return -1;
+ }
</span> if (ret == -1) {
stp->syserr = IOWAIT_SYSCALL;
stp->error_no = errno;
</code></pre> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=650022017-05-20T23:32:08Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p><a href="mailto:naruse@airemix.jp" class="email">naruse@airemix.jp</a> wrote:</p>
<blockquote>
<p>Only supports Linux:</p>
</blockquote>
<p>...And only with sockets where O_NONBLOCK is not set, because<br>
the O_NONBLOCK flag will ignore SO_*TIMEO.</p>
<p>Anyways, I don't think using SO_*TIMEO for Ruby is worth it.</p>
<p>I think Ruby can gain an internal timeout mechanism which can<br>
hook into all rb_<em>wait</em> APIs and raise Timeout::TimeoutError as<br>
appropriate.</p>
<p>I will consider that for my work-in-progress on auto-fibers;<br>
all APIs will have a timeout arg: <a href="https://blade.ruby-lang.org/ruby-core/81244">[ruby-core:81244]</a></p> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=723482018-06-02T08:46:56Znaruse (Yui NARUSE)naruse@airemix.jp
<ul></ul><p>I just noticed that just use write_nonblock can solve this ticket:</p>
<pre><code class="diff syntaxhl" data-language="diff"><span class="gh">diff --git a/lib/net/protocol.rb b/lib/net/protocol.rb
index 7ec636b384..2a806caeb6 100644
</span><span class="gd">--- a/lib/net/protocol.rb
</span><span class="gi">+++ b/lib/net/protocol.rb
</span><span class="p">@@ -77,6 +77,12 @@</span> class OpenTimeout < Timeout::Error; end
class ReadTimeout < Timeout::Error; end
<span class="gi">+ ##
+ # 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
+
</span>
class BufferedIO #:nodoc: internal use only
def initialize(io, read_timeout: 60, continue_timeout: nil, debug_output: nil)
<span class="p">@@ -237,9 +243,32 @@</span> def writing
def write0(*strs)
@debug_output << strs.map(&:dump).join if @debug_output
<span class="gd">- len = @io.write(*strs)
- @written_bytes += len
- len
</span><span class="gi">+ 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
</span> end
#
</code></pre> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=724132018-06-06T08:03:54Znaruse (Yui NARUSE)naruse@airemix.jp
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>Applied in changeset trunk|r63587.</p>
<hr>
<p>Introduce write_timeout to Net::HTTP [Feature <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Net::HTTP has no write timeout (Closed)" href="https://bugs.ruby-lang.org/issues/13396">#13396</a>]</p> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=724162018-06-06T09:04:38Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p><a href="mailto:naruse@ruby-lang.org" class="email">naruse@ruby-lang.org</a> wrote:</p>
<blockquote>
<p><a href="https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63587" class="external">https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63587</a></p>
</blockquote>
<p>For OpenSSL, I think you need to expect : wait_readable on<br>
write_nonblock, too.</p>
<p>Anyways, the code for handling partial write_nonblock case is verbose.<br>
One day, I would like to:</p>
<ol>
<li>integrate Timeout into core</li>
<li>make all SOCK_STREAM sockets non-blocking by default</li>
<li>Make rb_wait_for_single_fd aware of Timeouts</li>
</ol>
<p>So we can use:</p>
<p>Timeout.timeout(@write_timeout) { @io.write(strs) }</p>
<p>And no new background threads get spawned.</p>
<p>P.S.: If Ruby were LGPL-2.1+, I would steal the ccan/timer module which<br>
is optimized for frequently-expiring timers and be done with 1),<br>
already.</p> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=725132018-06-18T23:13:05Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>Eric Wong <a href="mailto:normalperson@yhbt.net" class="email">normalperson@yhbt.net</a> wrote:</p>
<blockquote>
<p>Anyways, the code for handling partial write_nonblock case is verbose.<br>
One day, I would like to:</p>
<ol>
<li>integrate Timeout into core</li>
<li>make all SOCK_STREAM sockets non-blocking by default</li>
<li>Make rb_wait_for_single_fd aware of Timeouts</li>
</ol>
</blockquote>
<p>FYI, I'm close to having a patch ready for 1) and 3);<br>
but maybe 3 is optional, even.</p>
<blockquote>
<p>So we can use:</p>
<pre><code>Timeout.timeout(@write_timeout) { @io.write(strs) }
</code></pre>
<p>And no new background threads get spawned.</p>
</blockquote>
<blockquote>
<p>P.S.: If Ruby were LGPL-2.1+, I would steal the ccan/timer module which<br>
is optimized for frequently-expiring timers and be done with 1),<br>
already.</p>
</blockquote>
<p>ccan/timer may not be the right tool for the job (more on this<br>
later).</p> Ruby master - Feature #13396: Net::HTTP has no write timeouthttps://bugs.ruby-lang.org/issues/13396?journal_id=925632021-06-17T13:33:20Zmiguelfteixeira (Miguel Teixeira)miguelfteixeira@gmail.com
<ul></ul><p>There's an open issue on <code>Net::HTTP#write_timeout</code> not working when <code>body_stream</code> is used: <a href="https://bugs.ruby-lang.org/issues/17933" class="external">https://bugs.ruby-lang.org/issues/17933</a></p>
<p>I'm just sharing this here for visibility.</p>