https://redmine.ruby-lang.org/https://redmine.ruby-lang.org/favicon.ico?17113305112011-04-07T22:23:06ZRuby Issue Tracking SystemRuby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163042011-04-07T22:23:06Zkosaki (Motohiro KOSAKI)kosaki.motohiro@gmail.com
<ul><li><strong>ruby -v</strong> changed from <i>ruby 1.9.3dev (2011-04-05 trunk 31241) [x86_64-darwin10.6.0]</i> to <i>-</i></li></ul><p>=begin</p>
<blockquote>
<hr>
<p>Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: TestSocket#test_closed_read fails after r31230 (Closed)" href="https://redmine.ruby-lang.org/issues/4558">#4558</a>: TestSocket#test_closed_read fails after r31230<br>
<a href="http://redmine.ruby-lang.org/issues/4558" class="external">http://redmine.ruby-lang.org/issues/4558</a></p>
</blockquote>
<p>I think current rb_io_close() is broken. We have to call rb_thread_fd_close()<br>
before releasing GVL.</p>
<p>Eric, Am I missing something?<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163102011-04-08T04:23:20Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>=begin<br>
KOSAKI Motohiro <a href="mailto:kosaki.motohiro@gmail.com" class="email">kosaki.motohiro@gmail.com</a> wrote:</p>
<blockquote>
<blockquote>
<hr>
<p>Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: TestSocket#test_closed_read fails after r31230 (Closed)" href="https://redmine.ruby-lang.org/issues/4558">#4558</a>: TestSocket#test_closed_read fails after r31230<br>
<a href="http://redmine.ruby-lang.org/issues/4558" class="external">http://redmine.ruby-lang.org/issues/4558</a></p>
</blockquote>
<p>I think current rb_io_close() is broken. We have to call rb_thread_fd_close()<br>
before releasing GVL.</p>
<p>Eric, Am I missing something?</p>
</blockquote>
<p>You are correct.</p>
<p>I can't reproduce the test failure on x86_64-linux but the<br>
following patch should fix a race condition:</p>
<p>diff --git a/io.c b/io.c<br>
index 7ce7148..b79cc5e 100644<br>
--- a/io.c<br>
+++ b/io.c<br>
@@ -3685,8 +3685,8 @@ rb_io_close(VALUE io)<br>
if (fptr->fd < 0) return Qnil;</p>
<pre><code> fd = fptr->fd;
</code></pre>
<ul>
<li>rb_io_fptr_cleanup(fptr, FALSE);<br>
rb_thread_fd_close(fd);</li>
</ul>
<ul>
<li>
<p>rb_io_fptr_cleanup(fptr, FALSE);</p>
<p>if (fptr->pid) {<br>
rb_syswait(fptr->pid);</p>
</li>
</ul>
<p>--<br>
Eric Wong<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163282011-04-09T01:23:06Zkosaki (Motohiro KOSAKI)kosaki.motohiro@gmail.com
<ul></ul><p>=begin<br>
2011/4/8 Eric Wong <a href="mailto:normalperson@yhbt.net" class="email">normalperson@yhbt.net</a>:</p>
<blockquote>
<p>KOSAKI Motohiro <a href="mailto:kosaki.motohiro@gmail.com" class="email">kosaki.motohiro@gmail.com</a> wrote:</p>
<blockquote>
<blockquote>
<hr>
<p>Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: TestSocket#test_closed_read fails after r31230 (Closed)" href="https://redmine.ruby-lang.org/issues/4558">#4558</a>: TestSocket#test_closed_read fails after r31230<br>
<a href="http://redmine.ruby-lang.org/issues/4558" class="external">http://redmine.ruby-lang.org/issues/4558</a></p>
</blockquote>
<p>I think current rb_io_close() is broken. We have to call rb_thread_fd_close()<br>
before releasing GVL.</p>
<p>Eric, Am I missing something?</p>
</blockquote>
<p>You are correct.</p>
<p>I can't reproduce the test failure on x86_64-linux but the<br>
following patch should fix a race condition:</p>
<p>diff --git a/io.c b/io.c<br>
index 7ce7148..b79cc5e 100644<br>
--- a/io.c<br>
+++ b/io.c<br>
@@ -3685,8 +3685,8 @@ rb_io_close(VALUE io)<br>
  if (fptr->fd < 0) return Qnil;</p>
<p>Â Â fd<br>
=end</p>
</blockquote> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163312011-04-09T09:15:46Znagachika (Tomoyuki Chikanaga)nagachika00@gmail.com
<ul><li><strong>Target version</strong> set to <i>1.9.3</i></li><li><strong>ruby -v</strong> changed from <i>-</i> to <i>ruby 1.9.3dev (2011-04-05 trunk 31241) [x86_64-darwin10.6.0]</i></li></ul><p>=begin<br>
Hi,</p>
<p>I applied Eric's patch, but TestSocket#test_closed_read still report same failure.</p>
<p>I can reproduce EBADF with following script.</p>
<p>r, w = IO.pipe<br>
read_thread = Thread.new{ r.read(1) }<br>
sleep(0.1) until read_thread.stop?<br>
r.close<br>
read_thread.join</p>
<p>=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163322011-04-09T12:23:06Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>=begin<br>
Tomoyuki Chikanaga <a href="mailto:redmine@ruby-lang.org" class="email">redmine@ruby-lang.org</a> wrote:</p>
<blockquote>
<p>ruby -v changed from - to ruby 1.9.3dev (2011-04-05 trunk 31241) [x86_64-darwin10.6.0]</p>
<p>Hi,</p>
<p>I applied Eric's patch, but TestSocket#test_closed_read still report same failure.</p>
<p>I can reproduce EBADF with following script.</p>
</blockquote>
<p>Thanks for testing, think I have a better fix below (supercedes my<br>
original fix)</p>
<p>Also pushed to the "io-close-fixes" branch of git://bogomips.org/ruby.git</p>
<p>From d5f9659ea9c2e8e0ed67544ed35afef4ca2bb3c5 Mon Sep 17 00:00:00 2001<br>
From: Eric Wong <a href="mailto:normalperson@yhbt.net" class="email">normalperson@yhbt.net</a><br>
Date: Thu, 7 Apr 2011 19:25:20 +0000<br>
Subject: [PATCH] io.c (rb_io_close): ensure IOError for cross-thread closes</p>
<p>We need to inform threads to stop operations on the FD before<br>
closing it and also invalidate the fd member of the rb_io_t<br>
struct for other threads to properly raise IOError.</p>
<p>FDs may be created and destroyed without the GVL, so<br>
rb_thread_fd_close() may be improperly hitting the wrong<br>
threads/FDs if we close() before notifying and in the worst case<br>
or threads will end up reading/writing to an unexpected FD.</p>
<h2>ref: <a href="/issues/4558">[ruby-core:35631]</a><br>
ref: <a href="http://redmine.ruby-lang.org/issues/4558" class="external">http://redmine.ruby-lang.org/issues/4558</a>
</h2>
<p>io.c | 25 ++++++++++++++++++-------<br>
test/ruby/test_io.rb | 39 +++++++++++++++++++++++++++++++++++++++<br>
2 files changed, 57 insertions(+), 7 deletions(-)</p>
<p>diff --git a/io.c b/io.c<br>
index 7ce7148..5d37b7f 100644<br>
--- a/io.c<br>
+++ b/io.c<br>
@@ -3504,6 +3504,7 @@ maygvl_close(int fd, int keepgvl)<br>
if (keepgvl)<br>
return close(fd);</p>
<ul>
<li>
<p>rb_thread_fd_close(fd);<br>
/*</p>
<ul>
<li>close() may block for certain file types (NFS, SO_LINGER sockets,</li>
<li>inotify), so let other threads run.<br>
@@ -3525,6 +3526,8 @@ maygvl_fclose(FILE *file, int keepgvl)<br>
if (keepgvl)<br>
return fclose(file);</li>
</ul>
</li>
<li>
<p>rb_thread_fd_close(fileno(file));</p>
</li>
<li>
<p>return (int)rb_thread_blocking_region(nogvl_fclose, file, RUBY_UBF_IO, 0);<br>
}</p>
</li>
</ul>
<p>@@ -3555,24 +3558,35 @@ fptr_finalize(rb_io_t *fptr, int noraise)<br>
}<br>
}<br>
if (IS_PREP_STDIO(fptr) || fptr->fd <= 2) {</p>
<ul>
<li>
<pre><code> int fd = fptr->fd;
</code></pre>
</li>
<li>
<li>
<pre><code> fptr->stdio_file = 0;
</code></pre>
</li>
<li>
<pre><code> fptr->fd = -1;
</code></pre>
</li>
<li>
<pre><code> rb_thread_fd_close(fd);
goto skip_fd_close;
</code></pre>
}<br>
if (fptr->stdio_file) {</li>
<li>FILE *stdio_file = fptr->stdio_file;</li>
<li>
<li>fptr->stdio_file = 0;</li>
<li>fptr->fd = -1;</li>
<li>
<pre><code> /* fptr->stdio_file is deallocated anyway
* even if fclose failed. */
</code></pre>
</li>
</ul>
<ul>
<li>if ((maygvl_fclose(fptr->stdio_file, noraise) < 0) && NIL_P(err))</li>
</ul>
<ul>
<li>if ((maygvl_fclose(stdio_file, noraise) < 0) && NIL_P(err))<br>
err = noraise ? Qtrue : INT2NUM(errno);<br>
}<br>
else if (0 <= fptr->fd) {</li>
<li>int fd = fptr->fd;</li>
<li>fptr->fd = -1;</li>
<li>
<pre><code> /* fptr->fd may be closed even if close fails.
* POSIX doesn't specify it.
* We assumes it is closed. */
</code></pre>
</li>
</ul>
<ul>
<li>if ((maygvl_close(fptr->fd, noraise) < 0) && NIL_P(err))</li>
</ul>
<ul>
<li>if ((maygvl_close(fd, noraise) < 0) && NIL_P(err))<br>
err = noraise ? Qtrue : INT2NUM(errno);<br>
}<br>
skip_fd_close:</li>
</ul>
<ul>
<li>
<p>fptr->fd = -1;</p>
</li>
<li>
<p>fptr->stdio_file = 0;<br>
fptr->mode &= ~(FMODE_READABLE|FMODE_WRITABLE);</p>
<p>if (!NIL_P(err) && !noraise) {<br>
@@ -3668,7 +3682,6 @@ VALUE<br>
rb_io_close(VALUE io)<br>
{<br>
rb_io_t *fptr;</p>
</li>
<li>
<p>int fd;<br>
VALUE write_io;<br>
rb_io_t *write_fptr;</p>
</li>
</ul>
<p>@@ -3684,9 +3697,7 @@ rb_io_close(VALUE io)<br>
if (!fptr) return Qnil;<br>
if (fptr->fd < 0) return Qnil;</p>
<ul>
<li>
<p>fd = fptr->fd;<br>
rb_io_fptr_cleanup(fptr, FALSE);</p>
</li>
<li>
<p>rb_thread_fd_close(fd);</p>
<p>if (fptr->pid) {<br>
rb_syswait(fptr->pid);<br>
diff --git a/test/ruby/test_io.rb b/test/ruby/test_io.rb<br>
index 6b8e6b5..3d086b3 100644<br>
--- a/test/ruby/test_io.rb<br>
+++ b/test/ruby/test_io.rb<br>
@@ -1809,4 +1809,43 @@ End<br>
Process.waitpid2(pid)<br>
end<br>
end</p>
</li>
</ul>
<ul>
<li>
<li>def test_cross_thread_close_fd</li>
<li>with_pipe do |r,w|</li>
<li>
<pre><code> read_thread = Thread.new do
</code></pre>
</li>
<li>
<pre><code> begin
</code></pre>
</li>
<li>
<pre><code> r.read(1)
</code></pre>
</li>
<li>
<pre><code> rescue => e
</code></pre>
</li>
<li>
<pre><code> e
</code></pre>
</li>
<li>
<pre><code> end
</code></pre>
</li>
<li>
<pre><code> end
</code></pre>
</li>
<li>
<li>
<pre><code> sleep(0.1) until read_thread.stop?
</code></pre>
</li>
<li>
<pre><code> r.close
</code></pre>
</li>
<li>
<pre><code> read_thread.join
</code></pre>
</li>
<li>
<pre><code> assert_kind_of(IOError, read_thread.value)
</code></pre>
</li>
<li>end</li>
<li>end</li>
<li>
<li>def test_cross_thread_close_stdio</li>
<li>with_pipe do |r,w|</li>
<li>
<pre><code> pid = fork do
</code></pre>
</li>
<li>
<pre><code> $stdin.reopen(r)
</code></pre>
</li>
<li>
<pre><code> r.close
</code></pre>
</li>
<li>
<pre><code> read_thread = Thread.new do
</code></pre>
</li>
<li>
<pre><code> begin
</code></pre>
</li>
<li>
<pre><code> $stdin.read(1)
</code></pre>
</li>
<li>
<pre><code> rescue => e
</code></pre>
</li>
<li>
<pre><code> e
</code></pre>
</li>
<li>
<pre><code> end
</code></pre>
</li>
<li>
<pre><code> end
</code></pre>
</li>
<li>
<pre><code> sleep(0.1) until read_thread.stop?
</code></pre>
</li>
<li>
<pre><code> $stdin.close
</code></pre>
</li>
<li>
<pre><code> read_thread.join
</code></pre>
</li>
<li>
<pre><code> exit(IOError === read_thread.value)
</code></pre>
</li>
<li>
<pre><code> end
</code></pre>
</li>
<li>
<pre><code> assert Process.waitpid2(pid)[1].success?
</code></pre>
</li>
<li>end</li>
<li>rescue NotImplementedError</li>
<li>end<br>
end<br>
--<br>
Eric Wong<br>
=end</li>
</ul> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163332011-04-09T12:23:06Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>=begin<br>
KOSAKI Motohiro <a href="mailto:kosaki.motohiro@gmail.com" class="email">kosaki.motohiro@gmail.com</a> wrote:</p>
<blockquote>
<p>After while thinking, I conclude I was wrong. If rb_io_fptr_cleanup()<br>
raise a exception, We don't have to kill other threads. So, now I'm<br>
incline to revert r31230. Hmm...</p>
</blockquote>
<p>I think we can still fix the issues revolving around r31230. We've<br>
already found at least two (this and <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: [PATCH] ext/socket/init.c: rsock_connect retries on interrupt (Closed)" href="https://redmine.ruby-lang.org/issues/4555">#4555</a>) bugs. As for <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: [PATCH] ext/socket/init.c: rsock_connect retries on interrupt (Closed)" href="https://redmine.ruby-lang.org/issues/4555">#4555</a><br>
(connect() failing on EINTR) it was a bug anyways, but just very hard to<br>
expose before.</p>
<p>--<br>
Eric Wong<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163342011-04-09T12:29:20Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>=begin<br>
Eric Wong <a href="mailto:normalperson@yhbt.net" class="email">normalperson@yhbt.net</a> wrote:</p>
<blockquote>
<p>Thanks for testing, think I have a better fix below (supercedes my<br>
original fix)</p>
<p>Also pushed to the "io-close-fixes" branch of git://bogomips.org/ruby.git</p>
</blockquote>
<p>Oops, I broke test/ruby/test_thread.rb in the atexit handlers :x<br>
I squashed the following change and forcibly repushed:</p>
<p>diff --git a/io.c b/io.c<br>
index 5d37b7f..65e7693 100644<br>
--- a/io.c<br>
+++ b/io.c<br>
@@ -3562,7 +3562,8 @@ fptr_finalize(rb_io_t *fptr, int noraise)</p>
<pre><code> fptr->stdio_file = 0;
fptr->fd = -1;
</code></pre>
<ul>
<li>
<pre><code> rb_thread_fd_close(fd);
</code></pre>
</li>
</ul>
<ul>
<li>
<pre><code> if (!noraise)
</code></pre>
</li>
<li>
<pre><code> rb_thread_fd_close(fd);
goto skip_fd_close;
</code></pre>
}<br>
if (fptr->stdio_file) {<br>
--<br>
Eric Wong<br>
=end</li>
</ul> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163362011-04-09T18:14:34Znagachika (Tomoyuki Chikanaga)nagachika00@gmail.com
<ul></ul><p>=begin<br>
Hi Eric,</p>
<p>These patches seem good and after applying them, make test-all passes all tests except a test for parallel_test in my environment. thanks!<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163432011-04-10T16:23:06Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>=begin<br>
Tomoyuki Chikanaga <a href="mailto:redmine@ruby-lang.org" class="email">redmine@ruby-lang.org</a> wrote:</p>
<blockquote>
<p>These patches seem good and after applying them, make test-all passes<br>
all tests except a test for parallel_test in my environment. thanks!</p>
</blockquote>
<p>Thanks for reporting and helping us track it down.<br>
Can somebody please commit my patches?</p>
<p>--<br>
Eric Wong<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163552011-04-11T21:53:16Zkosaki (Motohiro KOSAKI)kosaki.motohiro@gmail.com
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Closed</i></li></ul><p>=begin<br>
r31230 was revered by r31261.</p>
<p>=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163582011-04-11T22:23:06Zkosaki (Motohiro KOSAKI)kosaki.motohiro@gmail.com
<ul><li><strong>ruby -v</strong> changed from <i>ruby 1.9.3dev (2011-04-05 trunk 31241) [x86_64-darwin10.6.0]</i> to <i>-</i></li></ul><p>=begin</p>
<blockquote>
<p>Subject: [PATCH] io.c (rb_io_close): ensure IOError for cross-thread closes</p>
<p>We need to inform threads to stop operations on the FD before<br>
closing it and also invalidate the fd member of the rb_io_t<br>
struct for other threads to properly raise IOError.</p>
<p>FDs may be created and destroyed without the GVL, so<br>
rb_thread_fd_close() may be improperly hitting the wrong<br>
threads/FDs if we close() before notifying and in the worst case<br>
or threads will end up reading/writing to an unexpected FD.</p>
<h2>ref: <a href="/issues/4558">[ruby-core:35631]</a><br>
ref: <a href="http://redmine.ruby-lang.org/issues/4558" class="external">http://redmine.ruby-lang.org/issues/4558</a>
</h2>
<p> io.c         |  25 ++++++++++++++++++-------<br>
 test/ruby/test_io.rb |  39 +++++++++++++++++++++++++++++++++++++++<br>
 2 files changed, 57 insertions(+), 7 deletions(-)</p>
<p>diff --git a/io.c b/io.c<br>
index 7ce7148..5d37b7f 100644<br>
--- a/io.c<br>
+++ b/io.c<br>
@@ -3504,6 +3504,7 @@ maygvl_close(int fd, int keepgvl)<br>
  if (keepgvl)<br>
    return close(fd);</p>
<ul>
<li>
<p>Â Â rb_thread_fd_close(fd);<br>
  /*<br>
   * close() may block for certain file types (NFS, SO_LINGER sockets,<br>
   * inotify), so let other threads run.<br>
@@ -3525,6 +3526,8 @@ maygvl_fclose(FILE *file, int keepgvl)<br>
  if (keepgvl)<br>
    return fclose(file);</p>
</li>
<li>
<p>Â Â rb_thread_fd_close(fileno(file));</p>
</li>
<li>
</ul>
<p>Â Â return (int)rb_thread_blocking_region(nogvl_fclose, file, RUBY_UBF_IO, 0);<br>
 }</p>
<p>@@ -3555,24 +3558,35 @@ fptr_finalize(rb_io_t *fptr, int noraise)<br>
    }<br>
  }<br>
  if (IS_PREP_STDIO(fptr) || fptr->fd <<br>
=end</p>
</blockquote> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163642011-04-12T10:55:42Zusa (Usaku NAKAMURA)usa@garbagecollect.jp
<ul><li><strong>Category</strong> changed from <i>core</i> to <i>test</i></li><li><strong>Status</strong> changed from <i>Closed</i> to <i>Assigned</i></li><li><strong>Assignee</strong> set to <i>kosaki (Motohiro KOSAKI)</i></li></ul><p>=begin<br>
I have no opinion about this topic, but the test code which was checked in at r31260 by kosaki-san is platform dependent.<br>
It blocks on Windows, and stops all tests.</p>
<p>I request to revert it.<br>
Or, please explain grounds from which this test should be accepted as behavior of ruby.</p>
<p>=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=163652011-04-12T11:29:24Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>=begin<br>
I consider either Errno::EBADF or IOError to be acceptable.</p>
<p>The main thing I care about is I/O for pipes/sockets being interruptable<br>
(I only work on *nix).</p>
<p>By the way, test/socket/test_socket.rb has had a similar test for months<br>
(since r30852). It does have a Timeout wrapping it, so maybe that is<br>
needed (but you'd still get an error). Maybe just disabling this test<br>
for Windows platforms would be acceptable?</p>
<p>Eventually I would like to get rid of places where we call select()<br>
before doing (any) I/O across the board (ref: <a href="/issues/4538">[ruby-core:35586]</a>) if<br>
possible.</p>
<p>=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=164802011-04-12T22:23:07Zkosaki (Motohiro KOSAKI)kosaki.motohiro@gmail.com
<ul></ul><p>=begin<br>
2011/4/12 <a href="mailto:redmine@ruby-lang.org" class="email">redmine@ruby-lang.org</a>:</p>
<blockquote>
<p>Issue <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: TestSocket#test_closed_read fails after r31230 (Closed)" href="https://redmine.ruby-lang.org/issues/4558">#4558</a> has been updated by Usaku NAKAMURA.</p>
<p>Category changed from core to test<br>
Status changed from Closed to Assigned<br>
Assignee set to Motohiro KOSAKI</p>
<p>I have no opinion about this topic, but the test code which was checked in at r31260 by kosaki-san is platform dependent.<br>
It blocks on Windows, and stops all tests.</p>
<p>I request to revert it.<br>
Or, please explain grounds from which this test should be accepted as behavior of ruby.</p>
</blockquote>
<p>I succuseeded to reporoduce this issue. On win32, IO.close() cause hang-up.<br>
So, I think we have to discuss two thing.</p>
<ol>
<li>Why close() makes hang-up? Is it acceptable behavior?</li>
<li>At <a href="/issues/4390">[ruby-core:35203]</a>, We decided IO.close() raise exception to<br>
othread threads<br>
and then they should wake up as ruby-1.8.<br>
Should we think win32 is exception for this rule?</li>
</ol>
<p>In the other words, We have to decided rb_thread_io_blocking_region()<br>
spec. Otherwise<br>
we can't make any testcase. ;-)<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=164812011-04-12T22:23:07Zkosaki (Motohiro KOSAKI)kosaki.motohiro@gmail.com
<ul></ul><p>=begin</p>
<blockquote>
<p>Issue <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: TestSocket#test_closed_read fails after r31230 (Closed)" href="https://redmine.ruby-lang.org/issues/4558">#4558</a> has been updated by Eric Wong.</p>
<p>I consider either Errno::EBADF or IOError to be acceptable.</p>
</blockquote>
<p>Hmm...<br>
I can't agree this. If EBADF can be observed, we can observe completely<br>
unrelated file when a fd number was recycled just after close.</p>
<blockquote>
<p>The main thing I care about is I/O for pipes/sockets being interruptable<br>
(I only work on *nix).</p>
<p>By the way, test/socket/test_socket.rb has had a similar test for months<br>
(since r30852). Â It does have a Timeout wrapping it, so maybe that is<br>
needed (but you'd still get an error). Â Maybe just disabling this test<br>
for Windows platforms would be acceptable?</p>
</blockquote>
<p>Hmm...<br>
If windows can't implement close() case, I doubt r30852 is correct fix.<br>
Is this really worth that *nix and windows have different spec?</p>
<blockquote>
<p>Eventually I would like to get rid of places where we call select()<br>
before doing (any) I/O across the board (ref: <a href="/issues/4538">[ruby-core:35586]</a>) if<br>
possible.</p>
</blockquote>
<p>this makes sense to me.<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=164832011-04-13T13:29:07Zusa (Usaku NAKAMURA)usa@garbagecollect.jp
<ul></ul><p>=begin<br>
Hello,</p>
<p>In message "<a href="/issues/4558">[ruby-core:35725]</a> Re: [Ruby 1.9 - Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: TestSocket#test_closed_read fails after r31230 (Closed)" href="https://redmine.ruby-lang.org/issues/4558">#4558</a>][Assigned] TestSocket#test_closed_read fails after r31230"<br>
on Apr.12,2011 21:31:46, <a href="mailto:kosaki.motohiro@gmail.com" class="email">kosaki.motohiro@gmail.com</a> wrote:</p>
<blockquote>
<blockquote>
<p>Or, please explain grounds from which this test should be accepted as behavior of ruby.</p>
</blockquote>
<p>I succuseeded to reporoduce this issue. On win32, IO.close() cause hang-up.<br>
So, I think we have to discuss two thing.</p>
<ol>
<li>Why close() makes hang-up? Is it acceptable behavior?</li>
</ol>
</blockquote>
<p>MSVCRT's fds have their own locks.<br>
MSVCRT locks fds when accessing them -- reading, writing,<br>
closing, etc.<br>
The author of MSVCRT obviously intended the behavior, I think.</p>
<blockquote>
<ol start="2">
<li>At <a href="/issues/4390">[ruby-core:35203]</a>, We decided IO.close() raise exception to<br>
othread threads<br>
and then they should wake up as ruby-1.8.<br>
Should we think win32 is exception for this rule?</li>
</ol>
</blockquote>
<p>I see. Hmm...</p>
<p>Is the behavior that close() doesn't block and the I/O operations<br>
of other threads are interrupted defind by posix or some specifications?<br>
We found this problem in Windows this time, but might there be<br>
other platforms which have same problem?</p>
<a name="Regards"></a>
<h2 >Regards,<a href="#Regards" class="wiki-anchor">¶</a></h2>
<p>U.Nakamura <a href="mailto:usa@garbagecollect.jp" class="email">usa@garbagecollect.jp</a><br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=164852011-04-13T21:23:06Zkosaki (Motohiro KOSAKI)kosaki.motohiro@gmail.com
<ul></ul><p>=begin<br>
Hi</p>
<p>2011/4/13 U.Nakamura <a href="mailto:usa@garbagecollect.jp" class="email">usa@garbagecollect.jp</a>:</p>
<blockquote>
<p>Hello,</p>
<p>In message "<a href="/issues/4558">[ruby-core:35725]</a> Re: [Ruby 1.9 - Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: TestSocket#test_closed_read fails after r31230 (Closed)" href="https://redmine.ruby-lang.org/issues/4558">#4558</a>][Assigned] TestSocket#test_closed_read fails after r31230"<br>
  on Apr.12,2011 21:31:46, <a href="mailto:kosaki.motohiro@gmail.com" class="email">kosaki.motohiro@gmail.com</a> wrote:</p>
<blockquote>
<blockquote>
<p>Or, please explain grounds from which this test should be accepted as behavior of ruby.</p>
</blockquote>
<p>I succuseeded to reporoduce this issue. On win32, IO.close() cause hang-up.<br>
So, I think we have to discuss two thing.<br>
 1) Why close() makes hang-up? Is it acceptable behavior?</p>
</blockquote>
<p>MSVCRT's fds have their own locks.<br>
MSVCRT locks fds when accessing them -- reading, writing,<br>
closing, etc.<br>
The author of MSVCRT obviously intended the behavior, I think.</p>
</blockquote>
<p>ok, I see.</p>
<blockquote>
<blockquote>
<p>Â 2) At <a href="/issues/4390">[ruby-core:35203]</a>, We decided IO.close() raise exception to<br>
othread threads<br>
   and then they should wake up as ruby-1.8.<br>
   Should we think win32 is exception for this rule?</p>
</blockquote>
<p>I see. Â Hmm...</p>
<p>Is the behavior that close() doesn't block and the I/O operations<br>
of other threads are interrupted defind by posix or some specifications?</p>
</blockquote>
<p>No. It's purely implementation defined.</p>
<blockquote>
<p>We found this problem in Windows this time, but might there be<br>
other platforms which have same problem?</p>
</blockquote>
<p>It's possible.<br>
So, now I'm incline to revert r30852.</p>
<p>nobu, What do you think?<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=164932011-04-14T05:23:05Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>=begin<br>
KOSAKI Motohiro <a href="mailto:kosaki.motohiro@gmail.com" class="email">kosaki.motohiro@gmail.com</a> wrote:</p>
<blockquote>
<blockquote>
<p>Issue <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: TestSocket#test_closed_read fails after r31230 (Closed)" href="https://redmine.ruby-lang.org/issues/4558">#4558</a> has been updated by Eric Wong.</p>
<p>I consider either Errno::EBADF or IOError to be acceptable.</p>
</blockquote>
<p>Hmm...<br>
I can't agree this. If EBADF can be observed, we can observe completely<br>
unrelated file when a fd number was recycled just after close.</p>
</blockquote>
<p>Actually, I expect EBADF make any read/write-retry loop stop<br>
immediately, but yes, exposing IOError to user is better.</p>
<blockquote>
<p>Hmm...<br>
If windows can't implement close() case, I doubt r30852 is correct fix.<br>
Is this really worth that *nix and windows have different spec?</p>
</blockquote>
<p>If r30852 is reverted, Linux (and maybe other *nix) will still break<br>
threads out of blocking read/write with EBADF and<br>
rb_io_wait_*able(fptr->fd) will raise IOError as long as we<br>
assign fptr->fd = -1 before the actual close() call in IO#close.</p>
<p>Maybe Windows (and possibly other OS) will be forced to call do_select()<br>
to implement behavior consistent with Linux.</p>
<p>On a related note, r30852 also has the problem with making IO#close an<br>
O(n) operation since it needs to scan through all threads (and I'd like<br>
to run thousands of threads :>).</p>
<p>--<br>
Eric Wong<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=164982011-04-15T06:23:29Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>=begin<br>
KOSAKI Motohiro <a href="mailto:kosaki.motohiro@gmail.com" class="email">kosaki.motohiro@gmail.com</a> wrote:</p>
<blockquote>
<blockquote>
<p>KOSAKI Motohiro <a href="mailto:kosaki.motohiro@gmail.com" class="email">kosaki.motohiro@gmail.com</a> wrote:</p>
<blockquote>
<blockquote>
<p>Issue <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: TestSocket#test_closed_read fails after r31230 (Closed)" href="https://redmine.ruby-lang.org/issues/4558">#4558</a> has been updated by Eric Wong.<br>
following scenario should be happen too.</p>
</blockquote>
</blockquote>
</blockquote>
<a name="CPU1-CPU2-CPU3"></a>
<h2 >CPU1 CPU2 CPU3<a href="#CPU1-CPU2-CPU3" class="wiki-anchor">¶</a></h2>
<p>open() -> 5<br>
close(5)<br>
open() -> 5<br>
read(5) -> success, but read different data.</p>
</blockquote>
<p>OK, I'm starting to think there's no safe way to handle these<br>
situations, especially with MVM on the horizon. Just telling users to<br>
stop doing close() in a different thread is probably the best way to<br>
go...</p>
<blockquote>
<blockquote>
<blockquote>
<p>Hmm...<br>
If windows can't implement close() case, I doubt r30852 is correct fix.<br>
Is this really worth that *nix and windows have different spec?</p>
</blockquote>
<p>If r30852 is reverted, Linux (and maybe other *nix) will still break<br>
threads out of blocking read/write with EBADF and<br>
rb_io_wait_*able(fptr->fd) will raise IOError as long as we<br>
assign fptr->fd = -1 before the actual close() call in IO#close.</p>
<p>Maybe Windows (and possibly other OS) will be forced to call do_select()<br>
to implement behavior consistent with Linux.</p>
</blockquote>
<p>???<br>
I'm sorry, I haven't catch your point.<br>
Which issue is solved by calling do_select()?</p>
</blockquote>
<p>It can reduce the likelyhood of read() being uninterruptable since<br>
do_select() will sleep in 100ms intervals to check for interrupts on<br>
win. There's still a small chance of blocking read() since select()<br>
can have false positives and other threads could've drained the data...</p>
<blockquote>
<blockquote>
<p>On a related note, r30852 also has the problem with making IO#close an<br>
O(n) operation since it needs to scan through all threads (and I'd like<br>
to run thousands of threads :>).</p>
</blockquote>
<p>I have no opinion. I like faster software, but I haven't seen close<br>
makes performance bottleneck.</p>
</blockquote>
<p>Contrived test case, but it gets worse as nr_thread increases:</p>
<a name="ruby-192p180-2011-02-18-revision-30909-x86_64-linux"></a>
<h1 >ruby 1.9.2p180 (2011-02-18 revision 30909) [x86_64-linux]<a href="#ruby-192p180-2011-02-18-revision-30909-x86_64-linux" class="wiki-anchor">¶</a></h1>
<p>0.010000 0.000000 0.010000 ( 0.007288)</p>
<a name="ruby-193dev-2011-04-14-trunk-31267-x86_64-linux"></a>
<h1 >ruby 1.9.3dev (2011-04-14 trunk 31267) [x86_64-linux]<a href="#ruby-193dev-2011-04-14-trunk-31267-x86_64-linux" class="wiki-anchor">¶</a></h1>
<p>0.030000 0.000000 0.030000 ( 0.033881)<br>
-----------------------------8< -------------------------<br>
require "benchmark"<br>
nr_thread = 1_000<br>
nr_close = 1_000</p>
<h2>threads = nr_thread.times.map { Thread.new { sleep } }<br>
puts(Benchmark.measure do<br>
nr_close.times do<br>
File.open(<strong>FILE</strong>).close<br>
end<br>
end)<br>
threads.each { |thr| thr.run }.each { |thr| thr.join }</h2>
<p>Eric Wong<br>
=end</p> Ruby master - Bug #4558: TestSocket#test_closed_read fails after r31230https://redmine.ruby-lang.org/issues/4558?journal_id=177562011-06-11T16:20:59Zkosaki (Motohiro KOSAKI)kosaki.motohiro@gmail.com
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><ol>
<li>Windows test case failure have already been fixed.</li>
<li>"release GVL on close" is now discussing <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: [PATCH v2] io.c (rb_io_close): release GVL if possible (Closed)" href="https://redmine.ruby-lang.org/issues/4570">#4570</a>.</li>
</ol>
<p>So, we can close this ticket.</p>