From 514878d15c2be3590a73b458515037f8fe32f8c8 Mon Sep 17 00:00:00 2001 From: nagachika Date: Fri, 4 Oct 2013 17:30:50 +0000 Subject: [PATCH] merge revision(s) 43112,43114,43117,43118: [Backport #8980] * io.c (rb_io_close_read): keep fptr in write_io to be discarded, to fix freed pointer access when it is in use by other threads, and get rid of potential memory/fd leak. * io.c (rb_io_close_write): detach tied IO for writing before closing to get rid of race condition. [ruby-list:49598] * io.c (rb_io_close_read): duplex IO should wait its child process even after close_read. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_0_0@43144 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 11 +++++++++++ io.c | 13 +++++++++---- test/ruby/test_io.rb | 24 ++++++++++++++++++++++++ version.h | 2 +- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 94663f21bc..f74cd6fffb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +Sat Oct 5 02:14:56 2013 Nobuyoshi Nakada + + * io.c (rb_io_close_read): duplex IO should wait its child process + even after close_read. + +Sat Oct 5 02:14:56 2013 Nobuyoshi Nakada + + * io.c (rb_io_close_read): keep fptr in write_io to be discarded, to + fix freed pointer access when it is in use by other threads, and get + rid of potential memory/fd leak. + Sat Oct 5 01:59:50 2013 Shugo Maeda * vm_method.c (rb_undef): raise a NameError if the original method diff --git a/io.c b/io.c index 931bf45c79..23ab37144a 100644 --- a/io.c +++ b/io.c @@ -4369,11 +4369,16 @@ rb_io_close_read(VALUE io) write_io = GetWriteIO(io); if (io != write_io) { rb_io_t *wfptr; - rb_io_fptr_cleanup(fptr, FALSE); GetOpenFile(write_io, wfptr); + wfptr->pid = fptr->pid; + fptr->pid = 0; RFILE(io)->fptr = wfptr; - RFILE(write_io)->fptr = NULL; - rb_io_fptr_finalize(fptr); + /* bind to write_io temporarily to get rid of memory/fd leak */ + fptr->tied_io_for_writing = 0; + fptr->mode &= ~FMODE_DUPLEX; + RFILE(write_io)->fptr = fptr; + rb_io_fptr_cleanup(fptr, FALSE); + /* should not finalize fptr because another thread may be reading it */ return Qnil; } @@ -4429,12 +4434,12 @@ rb_io_close_write(VALUE io) rb_raise(rb_eIOError, "closing non-duplex IO for writing"); } - rb_io_close(write_io); if (io != write_io) { GetOpenFile(io, fptr); fptr->tied_io_for_writing = 0; fptr->mode &= ~FMODE_DUPLEX; } + rb_io_close(write_io); return Qnil; } diff --git a/test/ruby/test_io.rb b/test/ruby/test_io.rb index e0dfb24ba6..e7384390be 100644 --- a/test/ruby/test_io.rb +++ b/test/ruby/test_io.rb @@ -1218,6 +1218,19 @@ class TestIO < Test::Unit::TestCase end end + def test_close_read_write_separately + bug = '[ruby-list:49598]' + (1..10).each do |i| + assert_nothing_raised(IOError, "#{bug} trying ##{i}") do + IO.popen(EnvUtil.rubybin, "r+") {|f| + th = Thread.new {f.close_write} + f.close_read + th.join + } + end + end + end + def test_pid r, w = IO.pipe assert_equal(nil, r.pid) @@ -1234,6 +1247,17 @@ class TestIO < Test::Unit::TestCase assert_raise(IOError) { pipe.pid } end + def tesst_pid_after_close_read + pid1 = pid2 = nil + IO.popen(["echo", ""], "r+") do |io| + pid1 = io.pid + io.close_read + pid2 = io.pid + end + assert_not_nil(pid1) + assert_equal(pid1, pid2) + end + def make_tempfile t = Tempfile.new("test_io") t.binmode diff --git a/version.h b/version.h index 2c547d77a6..4c175a965c 100644 --- a/version.h +++ b/version.h @@ -1,6 +1,6 @@ #define RUBY_VERSION "2.0.0" #define RUBY_RELEASE_DATE "2013-10-05" -#define RUBY_PATCHLEVEL 325 +#define RUBY_PATCHLEVEL 326 #define RUBY_RELEASE_YEAR 2013 #define RUBY_RELEASE_MONTH 10