mame	2010-02-18 02:19:53 +0900 (Thu, 18 Feb 2010)

  New Revision: 26704

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=26704

  Log:
    * io.c (io_fread, io_getpartial, io_read, io_sysread): by using lock,
      prohibit modification of buffer string during read (which had caused
      EFAULT or SEGV).  [ruby-dev:40437]
    
    * test/ruby/test_io.rb: rewrite tests for the old behavior.

  Modified files:
    trunk/ChangeLog
    trunk/io.c
    trunk/test/ruby/test_io.rb

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 26703)
+++ ChangeLog	(revision 26704)
@@ -1,3 +1,11 @@
+Thu Feb 18 02:14:26 2010  Yusuke Endoh  <mame@t...>
+
+	* io.c (io_fread, io_getpartial, io_read, io_sysread): by using lock,
+	  prohibit modification of buffer string during read (which had caused
+	  EFAULT or SEGV).  [ruby-dev:40437]
+
+	* test/ruby/test_io.rb: rewrite tests for the old behavior.
+
 Wed Feb 17 21:34:01 2010  Yusuke Endoh  <mame@t...>
 
 	* regcomp.c (setup_tree, onig_compile): optimize .* at last by
Index: io.c
===================================================================
--- io.c	(revision 26703)
+++ io.c	(revision 26704)
@@ -1487,6 +1487,7 @@
     long n = len;
     long c;
 
+    rb_str_locktmp(str);
     if (READ_DATA_PENDING(fptr) == 0) {
 	while (n > 0) {
           again:
@@ -1504,6 +1505,7 @@
 	    if ((n -= c) <= 0) break;
 	    rb_thread_wait_fd(fptr->fd);
 	}
+	rb_str_unlocktmp(str);
 	return len - n;
     }
 
@@ -1519,6 +1521,7 @@
 	    break;
 	}
     }
+    rb_str_unlocktmp(str);
     return len - n;
 }
 
@@ -1810,18 +1813,15 @@
 
     if (!nonblock)
         READ_CHECK(fptr);
-    if (RSTRING_LEN(str) != len) {
-      modified:
-	rb_raise(rb_eRuntimeError, "buffer string modified");
-    }
     n = read_buffered_data(RSTRING_PTR(str), len, fptr);
     if (n <= 0) {
       again:
-	if (RSTRING_LEN(str) != len) goto modified;
         if (nonblock) {
             rb_io_set_nonblock(fptr);
         }
+	rb_str_locktmp(str);
 	n = rb_read_internal(fptr->fd, RSTRING_PTR(str), len);
+	rb_str_unlocktmp(str);
         if (n < 0) {
             if (!nonblock && rb_io_wait_readable(fptr->fd))
                 goto again;
@@ -2136,9 +2136,6 @@
     if (len == 0) return str;
 
     READ_CHECK(fptr);
-    if (RSTRING_LEN(str) != len) {
-	rb_raise(rb_eRuntimeError, "buffer string modified");
-    }
     n = io_fread(str, 0, fptr);
     if (n == 0) {
 	if (fptr->fd < 0) return Qnil;
@@ -3841,11 +3838,10 @@
     n = fptr->fd;
     rb_thread_wait_fd(fptr->fd);
     rb_io_check_closed(fptr);
-    if (RSTRING_LEN(str) != ilen) {
-	rb_raise(rb_eRuntimeError, "buffer string modified");
-    }
 
+    rb_str_locktmp(str);
     n = rb_read_internal(fptr->fd, RSTRING_PTR(str), ilen);
+    rb_str_unlocktmp(str);
 
     if (n == -1) {
 	rb_sys_fail_path(fptr->pathv);
Index: test/ruby/test_io.rb
===================================================================
--- test/ruby/test_io.rb	(revision 26703)
+++ test/ruby/test_io.rb	(revision 26704)
@@ -823,15 +823,15 @@
     end)
   end
 
-  def test_readpartial_error
+  def test_readpartial_lock
     with_pipe do |r, w|
       s = ""
       t = Thread.new { r.readpartial(5, s) }
       0 until s.size == 5
-      s.clear
+      assert_raise(RuntimeError) { s.clear }
       w.write "foobarbaz"
       w.close
-      assert_raise(RuntimeError) { t.join }
+      assert_equal("fooba", t.value)
     end
   end
 
@@ -858,15 +858,15 @@
     end)
   end
 
-  def test_read_error
+  def test_read_lock
     with_pipe do |r, w|
       s = ""
       t = Thread.new { r.read(5, s) }
       0 until s.size == 5
-      s.clear
+      assert_raise(RuntimeError) { s.clear }
       w.write "foobarbaz"
       w.close
-      assert_raise(RuntimeError) { t.join }
+      assert_equal("fooba", t.value)
     end
   end
 

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/