ruby-changes:38113
From: akr <ko1@a...>
Date: Thu, 9 Apr 2015 21:45:03 +0900 (JST)
Subject: [ruby-changes:38113] akr:r50194 (trunk): * internal.h (rb_execarg_parent_end): Declared.
akr 2015-04-09 21:44:35 +0900 (Thu, 09 Apr 2015) New Revision: 50194 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=50194 Log: * internal.h (rb_execarg_parent_end): Declared. * process.c: "spawn" opens files in the parent process. (check_exec_redirect): Add an placeholder for fd in parameters for fd_open. (check_exec_fds_1): Delete fd_open condition. (check_exec_fds): Don't call check_exec_fds_1 with fd_open. (rb_execarg_parent_start): Open files specified as "spawn" options and add "dup2" options. (rb_execarg_parent_end): New function to close opened fds. (run_exec_open): Removed. (rb_execarg_run_options): Don't call run_exec_open. (rb_spawn_internal): Call rb_execarg_parent_end. * io.c (pipe_open): Call rb_execarg_parent_end. * ext/pty/pty.c (establishShell): Call rb_execarg_parent_end. Modified files: trunk/ChangeLog trunk/ext/pty/pty.c trunk/internal.h trunk/io.c trunk/process.c trunk/test/ruby/test_process.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 50193) +++ ChangeLog (revision 50194) @@ -1,3 +1,23 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Thu Apr 9 21:38:20 2015 Tanaka Akira <akr@f...> + + * internal.h (rb_execarg_parent_end): Declared. + + * process.c: "spawn" opens files in the parent process. + (check_exec_redirect): Add an placeholder for fd in parameters + for fd_open. + (check_exec_fds_1): Delete fd_open condition. + (check_exec_fds): Don't call check_exec_fds_1 with fd_open. + (rb_execarg_parent_start): Open files specified as "spawn" options + and add "dup2" options. + (rb_execarg_parent_end): New function to close opened fds. + (run_exec_open): Removed. + (rb_execarg_run_options): Don't call run_exec_open. + (rb_spawn_internal): Call rb_execarg_parent_end. + + * io.c (pipe_open): Call rb_execarg_parent_end. + + * ext/pty/pty.c (establishShell): Call rb_execarg_parent_end. + Thu Apr 9 20:52:31 2015 Tanaka Akira <akr@f...> * internal.h (rb_execarg_parent_start): Renamed from rb_execarg_fixup. Index: io.c =================================================================== --- io.c (revision 50193) +++ io.c (revision 50194) @@ -6000,6 +6000,7 @@ pipe_open(VALUE execarg_obj, const char https://github.com/ruby/ruby/blob/trunk/io.c#L6000 if (0 <= arg.write_pair[1]) close(arg.write_pair[1]); if (0 <= arg.pair[0]) close(arg.pair[0]); if (0 <= arg.pair[1]) close(arg.pair[1]); + rb_execarg_parent_end(execarg_obj); rb_jump_tag(state); } @@ -6025,6 +6026,7 @@ pipe_open(VALUE execarg_obj, const char https://github.com/ruby/ruby/blob/trunk/io.c#L6026 if (eargp) rb_execarg_run_options(sargp, NULL, NULL, 0); # endif + rb_execarg_parent_end(execarg_obj); } else { # if defined(HAVE_WORKING_FORK) @@ -6083,8 +6085,10 @@ pipe_open(VALUE execarg_obj, const char https://github.com/ruby/ruby/blob/trunk/io.c#L6085 rb_execarg_run_options(eargp, sargp, NULL, 0); } fp = popen(cmd, modestr); - if (eargp) + if (eargp) { + rb_execarg_parent_end(execarg_obj); rb_execarg_run_options(sargp, NULL, NULL, 0); + } if (!fp) rb_sys_fail_path(prog); fd = fileno(fp); #endif Index: process.c =================================================================== --- process.c (revision 50193) +++ process.c (revision 50194) @@ -1514,8 +1514,8 @@ check_exec_redirect(VALUE key, VALUE val https://github.com/ruby/ruby/blob/trunk/process.c#L1514 flags = rb_to_int(flags); perm = rb_ary_entry(val, 2); perm = NIL_P(perm) ? INT2FIX(0644) : rb_to_int(perm); - param = hide_obj(rb_ary_new3(3, hide_obj(EXPORT_DUP(path)), - flags, perm)); + param = hide_obj(rb_ary_new3(4, hide_obj(EXPORT_DUP(path)), + flags, perm, Qnil)); eargp->fd_open = check_exec_redirect1(eargp->fd_open, key, param); } break; @@ -1542,8 +1542,8 @@ check_exec_redirect(VALUE key, VALUE val https://github.com/ruby/ruby/blob/trunk/process.c#L1542 else flags = INT2NUM(O_RDONLY); perm = INT2FIX(0644); - param = hide_obj(rb_ary_new3(3, hide_obj(EXPORT_DUP(path)), - flags, perm)); + param = hide_obj(rb_ary_new3(4, hide_obj(EXPORT_DUP(path)), + flags, perm, Qnil)); eargp->fd_open = check_exec_redirect1(eargp->fd_open, key, param); break; @@ -1769,7 +1769,7 @@ check_exec_fds_1(struct rb_execarg *earg https://github.com/ruby/ruby/blob/trunk/process.c#L1769 if (RTEST(rb_hash_lookup(h, INT2FIX(fd)))) { rb_raise(rb_eArgError, "fd %d specified twice", fd); } - if (ary == eargp->fd_open || ary == eargp->fd_dup2) + if (ary == eargp->fd_dup2) rb_hash_aset(h, INT2FIX(fd), Qtrue); else if (ary == eargp->fd_dup2_child) rb_hash_aset(h, INT2FIX(fd), RARRAY_AREF(elt, 1)); @@ -1797,7 +1797,6 @@ check_exec_fds(struct rb_execarg *eargp) https://github.com/ruby/ruby/blob/trunk/process.c#L1797 maxhint = check_exec_fds_1(eargp, h, maxhint, eargp->fd_dup2); maxhint = check_exec_fds_1(eargp, h, maxhint, eargp->fd_close); - maxhint = check_exec_fds_1(eargp, h, maxhint, eargp->fd_open); maxhint = check_exec_fds_1(eargp, h, maxhint, eargp->fd_dup2_child); if (eargp->fd_dup2_child) { @@ -2215,6 +2214,36 @@ rb_execarg_parent_start(VALUE execarg_ob https://github.com/ruby/ruby/blob/trunk/process.c#L2214 VALUE envopts; VALUE ary; + ary = eargp->fd_open; + if (ary != Qfalse) { + long i; + for (i = 0; i < RARRAY_LEN(ary); i++) { + VALUE elt = RARRAY_AREF(ary, i); + int fd = FIX2INT(RARRAY_AREF(elt, 0)); + VALUE param = RARRAY_AREF(elt, 1); + VALUE vpath = RARRAY_AREF(param, 0); + int flags = NUM2INT(RARRAY_AREF(param, 1)); + int perm = NUM2INT(RARRAY_AREF(param, 2)); + VALUE fd2v = RARRAY_AREF(param, 3); + int fd2; + if (NIL_P(fd2v)) { + const char *path; + FilePathValue(vpath); + path = StringValueCStr(vpath); + fd2 = rb_cloexec_open(path, flags, perm); + if (fd2 == -1) { + goto error; + } + rb_update_max_fd(fd2); + RARRAY_ASET(param, 3, INT2FIX(fd2)); + } + else { + fd2 = NUM2INT(fd2v); + } + rb_execarg_addopt(execarg_obj, INT2FIX(fd), INT2FIX(fd2)); + } + } + eargp->redirect_fds = check_exec_fds(eargp); ary = eargp->fd_dup2; @@ -2280,6 +2309,40 @@ rb_execarg_parent_start(VALUE execarg_ob https://github.com/ruby/ruby/blob/trunk/process.c#L2309 } */ } + + RB_GC_GUARD(execarg_obj); + return; + + error: + rb_execarg_parent_end(execarg_obj); + rb_sys_fail("open"); +} + +void +rb_execarg_parent_end(VALUE execarg_obj) +{ + struct rb_execarg *eargp = rb_execarg_get(execarg_obj); + int err = errno; + VALUE ary; + + ary = eargp->fd_open; + if (ary != Qfalse) { + long i; + for (i = 0; i < RARRAY_LEN(ary); i++) { + VALUE elt = RARRAY_AREF(ary, i); + VALUE param = RARRAY_AREF(elt, 1); + VALUE fd2v; + int fd2; + fd2v = RARRAY_AREF(param, 3); + if (!NIL_P(fd2v)) { + fd2 = FIX2INT(fd2v); + close(fd2); + RARRAY_ASET(param, 3, Qnil); + } + } + } + + errno = err; RB_GC_GUARD(execarg_obj); } @@ -2677,57 +2740,6 @@ run_exec_close(VALUE ary, char *errmsg, https://github.com/ruby/ruby/blob/trunk/process.c#L2740 /* This function should be async-signal-safe when sargp is NULL. Actually it is. */ static int -run_exec_open(VALUE ary, struct rb_execarg *sargp, char *errmsg, size_t errmsg_buflen) -{ - long i; - int ret; - - for (i = 0; i < RARRAY_LEN(ary);) { - VALUE elt = RARRAY_AREF(ary, i); - int fd = FIX2INT(RARRAY_AREF(elt, 0)); - VALUE param = RARRAY_AREF(elt, 1); - const VALUE vpath = RARRAY_AREF(param, 0); - const char *path = RSTRING_PTR(vpath); - int flags = NUM2INT(RARRAY_AREF(param, 1)); - int perm = NUM2INT(RARRAY_AREF(param, 2)); - int need_close = 1; - int fd2 = redirect_open(path, flags, perm); /* async-signal-safe */ - if (fd2 == -1) { - ERRMSG("open"); - return -1; - } - rb_update_max_fd(fd2); - while (i < RARRAY_LEN(ary) && - (elt = RARRAY_AREF(ary, i), RARRAY_AREF(elt, 1) == param)) { - fd = FIX2INT(RARRAY_AREF(elt, 0)); - if (fd == fd2) { - need_close = 0; - } - else { - if (save_redirect_fd(fd, sargp, errmsg, errmsg_buflen) < 0) /* async-signal-safe */ - return -1; - ret = redirect_dup2(fd2, fd); /* async-signal-safe */ - if (ret == -1) { - ERRMSG("dup2"); - return -1; - } - rb_update_max_fd(fd); - } - i++; - } - if (need_close) { - ret = redirect_close(fd2); /* async-signal-safe */ - if (ret == -1) { - ERRMSG("close"); - return -1; - } - } - } - return 0; -} - -/* This function should be async-signal-safe when sargp is NULL. Actually it is. */ -static int run_exec_dup2_child(VALUE ary, struct rb_execarg *sargp, char *errmsg, size_t errmsg_buflen) { long i; @@ -2927,12 +2939,6 @@ rb_execarg_run_options(const struct rb_e https://github.com/ruby/ruby/blob/trunk/process.c#L2939 } #endif - obj = eargp->fd_open; - if (obj != Qfalse) { - if (run_exec_open(obj, sargp, errmsg, errmsg_buflen) == -1) /* async-signal-safe */ - return -1; - } - obj = eargp->fd_dup2_child; if (obj != Qfalse) { if (run_exec_dup2_child(obj, sargp, errmsg, errmsg_buflen) == -1) /* async-signal-safe */ @@ -3848,6 +3854,7 @@ rb_spawn_internal(int argc, const VALUE https://github.com/ruby/ruby/blob/trunk/process.c#L3854 eargp = rb_execarg_get(execarg_obj); rb_execarg_parent_start(execarg_obj); ret = rb_spawn_process(eargp, errmsg, errmsg_buflen); + rb_execarg_parent_end(execarg_obj); RB_GC_GUARD(execarg_obj); return ret; } @@ -4215,6 +4222,7 @@ rb_f_spawn(int argc, VALUE *argv) https://github.com/ruby/ruby/blob/trunk/process.c#L4222 fail_str = eargp->use_shell ? eargp->invoke.sh.shell_script : eargp->invoke.cmd.command_name; pid = rb_spawn_process(eargp, errmsg, sizeof(errmsg)); + rb_execarg_parent_end(execarg_obj); RB_GC_GUARD(execarg_obj); if (pid == -1) { Index: ext/pty/pty.c =================================================================== --- ext/pty/pty.c (revision 50193) +++ ext/pty/pty.c (revision 50194) @@ -196,12 +196,14 @@ establishShell(int argc, VALUE *argv, st https://github.com/ruby/ruby/blob/trunk/ext/pty/pty.c#L196 int e = errno; close(master); close(slave); + rb_execarg_parent_end(carg.execarg_obj); errno = e; if (status) rb_jump_tag(status); rb_sys_fail(errbuf[0] ? errbuf : "fork failed"); } close(slave); + rb_execarg_parent_end(carg.execarg_obj); info->child_pid = pid; info->fd = master; Index: internal.h =================================================================== --- internal.h (revision 50193) +++ internal.h (revision 50194) @@ -1252,6 +1252,7 @@ struct rb_execarg *rb_execarg_get(VALUE https://github.com/ruby/ruby/blob/trunk/internal.h#L1252 VALUE rb_execarg_init(int argc, const VALUE *argv, int accept_shell, VALUE execarg_obj); int rb_execarg_addopt(VALUE execarg_obj, VALUE key, VALUE val); void rb_execarg_parent_start(VALUE execarg_obj); +void rb_execarg_parent_end(VALUE execarg_obj); int rb_execarg_run_options(const struct rb_execarg *e, struct rb_execarg *s, char* errmsg, size_t errmsg_buflen); VALUE rb_execarg_extract_options(VALUE execarg_obj, VALUE opthash); void rb_execarg_setenv(VALUE execarg_obj, VALUE env); Index: test/ruby/test_process.rb =================================================================== --- test/ruby/test_process.rb (revision 50193) +++ test/ruby/test_process.rb (revision 50194) @@ -536,6 +536,28 @@ class TestProcess < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_process.rb#L536 } end + def test_execopts_redirect_open_order_normal + minfd = 3 + maxfd = 20 + with_tmpchdir {|d| + opts = {} + minfd.upto(maxfd) {|fd| opts[fd] = ["out#{fd}", "w"] } + system RUBY, "-e", "#{minfd}.upto(#{maxfd}) {|fd| IO.new(fd).print fd.to_s }", opts + minfd.upto(maxfd) {|fd| assert_equal(fd.to_s, File.read("out#{fd}")) } + } + end + + def test_execopts_redirect_open_order_reverse + minfd = 3 + maxfd = 20 + with_tmpchdir {|d| + opts = {} + maxfd.downto(minfd) {|fd| opts[fd] = ["out#{fd}", "w"] } + system RUBY, "-e", "#{minfd}.upto(#{maxfd}) {|fd| IO.new(fd).print fd.to_s }", opts + minfd.upto(maxfd) {|fd| assert_equal(fd.to_s, File.read("out#{fd}")) } + } + end + def test_execopts_redirect_pipe with_pipe {|r1, w1| with_pipe {|r2, w2| -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/