From ac40a4814a99fd0cda5313f753ce15562959be60 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 30 Nov 2017 20:16:54 -0600 Subject: [PATCH] Mutiple fixes for pipes and subprocess IO. * Port 2015 redirect logic for popen. * Improve cloexec for subprocess IO. * Make cloexec work for any streams we can get realFileno for. ** This allows it to work for JVM-provided pipes if they can be unwrapped. See #4858 --- core/src/main/java/org/jruby/RubyIO.java | 45 +-- core/src/main/java/org/jruby/api/API.java | 58 ++++ .../java/org/jruby/util/io/ChannelFD.java | 2 +- .../main/java/org/jruby/util/io/OpenFile.java | 27 +- .../java/org/jruby/util/io/PopenExecutor.java | 280 +++++++++++------- .../java/org/jruby/util/io/PosixShim.java | 11 + 6 files changed, 292 insertions(+), 131 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyIO.java b/core/src/main/java/org/jruby/RubyIO.java index afa3f49431a..eea941a0945 100644 --- a/core/src/main/java/org/jruby/RubyIO.java +++ b/core/src/main/java/org/jruby/RubyIO.java @@ -644,10 +644,8 @@ public IRubyObject reopen(ThreadContext context, IRubyObject[] args) { // } fptr.setFD(sysopen(runtime, fptr.getPath(), oflags_p[0], 0666)); - // This logic fixes the original stdio file descriptor by clearing any CLOEXEC that might have - // come across with the newly opened file. Since we do not yet support CLOEXEC, we skip this. - // fptr.fd = fileno(fptr.stdio_file); - // rb_fd_fix_cloexec(fptr.fd); +// fptr.fd = fileno(fptr.stdio_file); + OpenFile.fdFixCloexec(fptr.posix, fptr.fd().realFileno); // This logic configures buffering (none, line, full) and buffer size to match the original stdio // stream associated with this IO. I don't believe we can do this. @@ -1212,11 +1210,11 @@ else if (!(intmode = TypeConverter.checkIntegerType(context, vmode)).isNil()) return runtime.newFixnum(fd.bestFileno()); } - private static class Sysopen { - String fname; - int oflags; - int perm; - Errno errno; + public static class Sysopen { + public String fname; + public int oflags; + public int perm; + public Errno errno; } // rb_sysopen @@ -1255,8 +1253,7 @@ private static ChannelFD sysopenFunc(Ruby runtime, Sysopen data) { } // rb_cloexec_open - private static ChannelFD cloexecOpen(Ruby runtime, Sysopen data) - { + public static ChannelFD cloexecOpen(Ruby runtime, Sysopen data) { Channel ret = null; // #ifdef O_CLOEXEC // /* O_CLOEXEC is available since Linux 2.6.23. Linux 2.6.18 silently ignore it. */ @@ -1270,9 +1267,10 @@ private static ChannelFD cloexecOpen(Ruby runtime, Sysopen data) data.errno = shim.errno; return null; } - // TODO, if we need it? -// rb_maygvl_fd_fix_cloexec(ret); - return new ChannelFD(ret, runtime.getPosix(), runtime.getFilenoUtil()); + ChannelFD fd = new ChannelFD(ret, runtime.getPosix(), runtime.getFilenoUtil()); + OpenFile.fdFixCloexec(shim, fd.realFileno); + + return fd; } // MRI: rb_io_autoclose_p @@ -2176,19 +2174,21 @@ public IRubyObject close_on_exec_set(ThreadContext context, IRubyObject arg) { POSIX posix = runtime.getPosix(); OpenFile fptr = getOpenFileChecked(); RubyIO write_io; + int fd = -1; - if (fptr.fd().chNative == null || !posix.isNative()) { + if (fptr == null || (fd = fptr.fd().realFileno) == -1 + || !posix.isNative()) { runtime.getWarnings().warning("close_on_exec is not implemented for this stream type: " + fptr.fd().ch.getClass().getSimpleName()); return context.nil; } int flag = arg.isTrue() ? FD_CLOEXEC : 0; - int fd, ret; + int ret; write_io = GetWriteIO(); if (this != write_io) { fptr = write_io.getOpenFileChecked(); - if (fptr != null && 0 <= (fd = fptr.fd().chNative.getFD())) { + if (fptr != null && 0 <= (fd = fptr.fd().realFileno)) { if ((ret = posix.fcntl(fd, Fcntl.F_GETFD)) == -1) return API.rb_sys_fail_path(runtime, fptr.getPath()); if ((ret & FD_CLOEXEC) != flag) { ret = (ret & ~FD_CLOEXEC) | flag; @@ -2200,7 +2200,7 @@ public IRubyObject close_on_exec_set(ThreadContext context, IRubyObject arg) { } fptr = getOpenFileChecked(); - if (fptr != null && 0 <= (fd = fptr.fd().chNative.getFD())) { + if (fptr != null && 0 <= (fd = fptr.fd().realFileno)) { if ((ret = posix.fcntl(fd, Fcntl.F_GETFD)) == -1) API.rb_sys_fail_path(runtime, fptr.getPath()); if ((ret & FD_CLOEXEC) != flag) { ret = (ret & ~FD_CLOEXEC) | flag; @@ -2217,26 +2217,27 @@ public IRubyObject close_on_exec_p(ThreadContext context) { Ruby runtime = context.runtime; POSIX posix = runtime.getPosix(); OpenFile fptr = getOpenFileChecked(); + int fd = -1; - if (fptr == null || fptr.fd().chNative == null + if (fptr == null || (fd = fptr.fd().realFileno) == -1 || !posix.isNative()) { return context.fals; } RubyIO write_io; - int fd, ret; + int ret; write_io = GetWriteIO(); if (this != write_io) { fptr = write_io.getOpenFileChecked(); - if (fptr != null && 0 <= (fd = fptr.fd().chNative.getFD())) { + if (fptr != null && 0 <= (fd = fptr.fd().realFileno)) { if ((ret = posix.fcntl(fd, Fcntl.F_GETFD)) == -1) API.rb_sys_fail_path(runtime, fptr.getPath()); if ((ret & FD_CLOEXEC) == 0) return context.fals; } } fptr = getOpenFileChecked(); - if (fptr != null && 0 <= (fd = fptr.fd().chNative.getFD())) { + if (fptr != null && 0 <= (fd = fptr.fd().realFileno)) { if ((ret = posix.fcntl(fd, Fcntl.F_GETFD)) == -1) API.rb_sys_fail_path(runtime, fptr.getPath()); if ((ret & FD_CLOEXEC) == 0) return context.fals; } diff --git a/core/src/main/java/org/jruby/api/API.java b/core/src/main/java/org/jruby/api/API.java index 04fb31af208..1546df4854c 100644 --- a/core/src/main/java/org/jruby/api/API.java +++ b/core/src/main/java/org/jruby/api/API.java @@ -1,10 +1,68 @@ package org.jruby.api; import org.jruby.Ruby; +import org.jruby.RubyIO; import org.jruby.runtime.builtin.IRubyObject; +import org.jruby.util.io.OpenFile; +import org.jruby.util.io.PosixShim; public class API { public static IRubyObject rb_sys_fail_path(Ruby runtime, String path) { throw runtime.newSystemCallError("bad path for cloexec: " + path); } + + public static int rb_pipe(Ruby runtime, int[] pipes) { + int ret; + ret = rb_cloexec_pipe(runtime, pipes); +// if (ret == -1) { +// if (rb_gc_for_fd(errno)) { +// ret = rb_cloexec_pipe(pipes); +// } +// } +// if (ret == 0) { +// rb_update_max_fd(pipes[0]); +// rb_update_max_fd(pipes[1]); +// } + return ret; + } + + public static int rb_cloexec_pipe(Ruby runtime, int[] pipes) { + int ret; + +//#if defined(HAVE_PIPE2) +// static int try_pipe2 = 1; +// if (try_pipe2) { +// ret = runtime.posix.pippipe2(fildes, O_CLOEXEC); +// if (ret != -1) +// return ret; +// /* pipe2 is available since Linux 2.6.27, glibc 2.9. */ +// if (errno == ENOSYS) { +// try_pipe2 = 0; +// ret = pipe(fildes); +// } +// } +// else { +// ret = pipe(fildes); +// } +//#else + ret = runtime.getPosix().pipe(pipes); +//#endif + if (ret == -1) return -1; +//#ifdef __CYGWIN__ +// if (ret == 0 && fildes[1] == -1) { +// close(fildes[0]); +// fildes[0] = -1; +// errno = ENFILE; +// return -1; +// } +//#endif + rb_maygvl_fd_fix_cloexec(runtime, pipes[0]); + rb_maygvl_fd_fix_cloexec(runtime, pipes[1]); + return ret; + } + + public static void rb_maygvl_fd_fix_cloexec(Ruby runtime, int fd) { + PosixShim shim = new PosixShim(runtime); + OpenFile.fdFixCloexec(shim, fd); + } } diff --git a/core/src/main/java/org/jruby/util/io/ChannelFD.java b/core/src/main/java/org/jruby/util/io/ChannelFD.java index 541ec09ab02..d28d5147342 100644 --- a/core/src/main/java/org/jruby/util/io/ChannelFD.java +++ b/core/src/main/java/org/jruby/util/io/ChannelFD.java @@ -84,7 +84,7 @@ public int dup2From(POSIX posix, ChannelFD dup2Source) { this.currentLock = dup2Source.currentLock; - return 0; + return fakeFileno; } public void close() throws IOException { diff --git a/core/src/main/java/org/jruby/util/io/OpenFile.java b/core/src/main/java/org/jruby/util/io/OpenFile.java index f6d56a7a220..1c57cd309d3 100644 --- a/core/src/main/java/org/jruby/util/io/OpenFile.java +++ b/core/src/main/java/org/jruby/util/io/OpenFile.java @@ -1,6 +1,7 @@ package org.jruby.util.io; import jnr.constants.platform.Errno; +import jnr.constants.platform.Fcntl; import jnr.constants.platform.OpenFlags; import org.jcodings.Encoding; import org.jcodings.Ptr; @@ -19,6 +20,7 @@ import org.jruby.RubyString; import org.jruby.RubyThread; import org.jruby.exceptions.RaiseException; +import org.jruby.ext.fcntl.FcntlLibrary; import org.jruby.platform.Platform; import org.jruby.runtime.Block; import org.jruby.runtime.ThreadContext; @@ -2575,11 +2577,32 @@ public static int cloexecDup2(PosixShim posix, ChannelFD oldfd, ChannelFD newfd) // #endif if (ret == -1) return -1; } - // TODO? -// rb_maygvl_fd_fix_cloexec(ret); + fdFixCloexec(posix, ret); return ret; } + // MRI: rb_maygvl_fd_fix_cloexec, without compiler conditions + public static void fdFixCloexec(PosixShim posix, int fd) { + if (fd >= 0 && fd < FilenoUtil.FIRST_FAKE_FD) { + int flags, flags2, ret; + flags = posix.fcntlGetFD(fd); /* should not fail except EBADF. */ + if (flags == -1) { + throw new RuntimeException(String.format("BUG: rb_maygvl_fd_fix_cloexec: fcntl(%d, F_GETFD) failed: %s", fd, posix.errno.description())); + } + if (fd <= 2) + flags2 = flags & ~FcntlLibrary.FD_CLOEXEC; /* Clear CLOEXEC for standard file descriptors: 0, 1, 2. */ + else + flags2 = flags | FcntlLibrary.FD_CLOEXEC; /* Set CLOEXEC for non-standard file descriptors: 3, 4, 5, ... */ + if (flags != flags2) { + ret = posix.fcntlSetFD(fd, flags2); + if (ret == -1) { + throw new RuntimeException(String.format("BUG: rb_maygvl_fd_fix_cloexec: fcntl(%d, F_SETFD) failed: %s", fd, flags2, posix.errno.description())); + } + } + } + // otherwise JVM sets cloexec + } + /** * Add a thread to the list of blocking threads for this IO. * diff --git a/core/src/main/java/org/jruby/util/io/PopenExecutor.java b/core/src/main/java/org/jruby/util/io/PopenExecutor.java index 511a462e888..8042075a7e2 100644 --- a/core/src/main/java/org/jruby/util/io/PopenExecutor.java +++ b/core/src/main/java/org/jruby/util/io/PopenExecutor.java @@ -1,7 +1,10 @@ package org.jruby.util.io; import jnr.constants.platform.Errno; +import jnr.constants.platform.Fcntl; import jnr.constants.platform.OpenFlags; +import jnr.enxio.channels.NativeDeviceChannel; +import jnr.enxio.channels.NativeSelectableChannel; import jnr.posix.SpawnAttribute; import jnr.posix.SpawnFileAction; import org.jcodings.transcode.EConvFlags; @@ -17,7 +20,10 @@ import org.jruby.RubyProcess; import org.jruby.RubyString; import org.jruby.RubySymbol; +import org.jruby.api.API; import org.jruby.common.IRubyWarnings; +import org.jruby.exceptions.RaiseException; +import org.jruby.ext.fcntl.FcntlLibrary; import org.jruby.platform.Platform; import org.jruby.runtime.Arity; import org.jruby.runtime.Block; @@ -514,8 +520,8 @@ private IRubyObject pipeOpen(ThreadContext context, ExecArg eargp, String modest String[] envp = null; ExecArg sargp = new ExecArg(); - Channel fd; - Channel write_fd = null; + int fd; + int write_fd = -1; String cmd = null; if (prog != null) @@ -532,41 +538,50 @@ private IRubyObject pipeOpen(ThreadContext context, ExecArg eargp, String modest if (eargp != null && !eargp.use_shell) { args = eargp.argv_str.argv; } - Channel[] mainPipe = null, secondPipe = null; + int[] pair = {-1,-1}, writePair = {-1, -1}; switch (fmode & (OpenFile.READABLE|OpenFile.WRITABLE)) { case OpenFile.READABLE | OpenFile.WRITABLE: - if ((secondPipe = posix.pipe()) == null) + if (API.rb_pipe(runtime, writePair) == -1) throw runtime.newErrnoFromErrno(posix.errno, prog.toString()); - if ((mainPipe = posix.pipe()) == null) { + if (API.rb_pipe(runtime, pair) == -1) { e = posix.errno; - try {secondPipe[1].close();} catch (IOException ioe) {} - try {secondPipe[0].close();} catch (IOException ioe) {} + runtime.getPosix().close(writePair[1]); + runtime.getPosix().close(writePair[0]); posix.errno = e; throw runtime.newErrnoFromErrno(posix.errno, prog.toString()); } - if (eargp != null) prepareStdioRedirects(runtime, mainPipe, secondPipe, eargp); + if (eargp != null) prepareStdioRedirects(runtime, pair, writePair, eargp); break; case OpenFile.READABLE: - if ((mainPipe = posix.pipe()) == null) + if (API.rb_pipe(runtime, pair) == -1) throw runtime.newErrnoFromErrno(posix.errno, prog.toString()); - if (eargp != null) prepareStdioRedirects(runtime, mainPipe, null, eargp); + if (eargp != null) prepareStdioRedirects(runtime, pair, null, eargp); break; case OpenFile.WRITABLE: - if ((mainPipe = posix.pipe()) == null) + if (API.rb_pipe(runtime, pair) == -1) throw runtime.newErrnoFromErrno(posix.errno, prog.toString()); - if (eargp != null) prepareStdioRedirects(runtime, null, mainPipe, eargp); + if (eargp != null) prepareStdioRedirects(runtime, null, pair, eargp); break; default: throw runtime.newSystemCallError(prog.toString()); } if (eargp != null) { - execargFixup(context, runtime, eargp); + try { + execargFixup(context, runtime, eargp); + } catch (RaiseException re) { // if (state) + if (writePair[0] != -1) runtime.getPosix().close(writePair[0]); + if (writePair[1] != -1) runtime.getPosix().close(writePair[1]); + if (pair[0] != -1) runtime.getPosix().close(pair[0]); + if (pair[1] != -1) runtime.getPosix().close(pair[1]); + execargParentEnd(runtime, eargp); + throw re; + } execargRunOptions(context, runtime, eargp, sargp, null); if (eargp.envp_str != null) envp = eargp.envp_str; while ((pid = DO_SPAWN(runtime, eargp, cmd, args, envp)) == -1) { @@ -579,9 +594,9 @@ private IRubyObject pipeOpen(ThreadContext context, ExecArg eargp, String modest } break; } - // We don't modify parent, so nothing to fix up -// if (eargp != null) -// execargRunOptions(context, runtime, sargp, null, null); + if (eargp != null) + execargRunOptions(context, runtime, sargp, null, null); + execargParentEnd(runtime, eargp); } else { throw runtime.newNotImplementedError("spawn without exec args (probably a bug)"); @@ -589,33 +604,33 @@ private IRubyObject pipeOpen(ThreadContext context, ExecArg eargp, String modest /* parent */ if (pid == -1) { - try {mainPipe[1].close();} catch (IOException ioe) {} - try {mainPipe[0].close();} catch (IOException ioe) {} + runtime.getPosix().close(pair[1]); + runtime.getPosix().close(pair[0]); if ((fmode & (OpenFile.READABLE|OpenFile.WRITABLE)) == (OpenFile.READABLE|OpenFile.WRITABLE)) { - try {mainPipe[1].close();} catch (IOException ioe) {} - try {mainPipe[0].close();} catch (IOException ioe) {} + runtime.getPosix().close(pair[1]); + runtime.getPosix().close(pair[0]); } errno = e; throw runtime.newErrnoFromErrno(errno, prog.toString()); } if ((fmode & OpenFile.READABLE) != 0 && (fmode & OpenFile.WRITABLE) != 0) { - try {mainPipe[1].close();} catch (IOException ioe) {} - fd = mainPipe[0]; - try {secondPipe[0].close();} catch (IOException ioe) {} - write_fd = secondPipe[1]; + runtime.getPosix().close(pair[1]); + fd = pair[0]; + runtime.getPosix().close(writePair[0]); + write_fd = writePair[1]; } else if ((fmode & OpenFile.READABLE) != 0) { - try {mainPipe[1].close();} catch (IOException ioe) {} - fd = mainPipe[0]; + runtime.getPosix().close(pair[1]); + fd = pair[0]; } else { - try {mainPipe[0].close();} catch (IOException ioe) {} - fd = mainPipe[1]; + runtime.getPosix().close(pair[0]); + fd = pair[1]; } port = runtime.getIO().allocate(); fptr = ((RubyIO)port).MakeOpenFile(); - fptr.setChannel(fd); + fptr.setChannel(new NativeDeviceChannel(fd)); fptr.setMode(fmode | (OpenFile.SYNC|OpenFile.DUPLEX)); if (convconfig != null) { fptr.encs.copy(convconfig); @@ -639,10 +654,10 @@ else if ((fmode & OpenFile.READABLE) != 0) { fptr.setPid(pid); fptr.setProcess(new POSIXProcess(runtime, finalPid)); - if (write_fd != null) { + if (write_fd != -1) { write_port = runtime.getIO().allocate(); write_fptr = ((RubyIO)write_port).MakeOpenFile(); - write_fptr.setChannel(write_fd); + write_fptr.setChannel(new NativeDeviceChannel(write_fd)); write_fptr.setMode((fmode & ~OpenFile.READABLE)| OpenFile.SYNC|OpenFile.DUPLEX); fptr.setMode(fptr.getMode() & ~OpenFile.WRITABLE); fptr.tiedIOForWriting = (RubyIO)write_port; @@ -656,7 +671,7 @@ else if ((fmode & OpenFile.READABLE) != 0) { return port; } - private void prepareStdioRedirects(Ruby runtime, Channel[] readPipe, Channel[] writePipe, ExecArg eargp) { + private void prepareStdioRedirects(Ruby runtime, int[] readPipe, int[] writePipe, ExecArg eargp) { // We insert these redirects directly into fd_dup2 so that chained redirection can be // validated and set up properly by the execargFixup logic. // The closes do not appear to be part of MRI's logic (they close the fd before exec/spawn), @@ -664,21 +679,21 @@ private void prepareStdioRedirects(Ruby runtime, Channel[] readPipe, Channel[] w if (readPipe != null) { // dup our read pipe's write end into stdout - int readPipeWriteFD = FilenoUtil.filenoFrom(readPipe[1]); + int readPipeWriteFD = readPipe[1]; eargp.fd_dup2 = checkExecRedirect1(runtime, eargp.fd_dup2, runtime.newFixnum(1), runtime.newFixnum(readPipeWriteFD)); // close the other end of the pipe in the child - int readPipeReadFD = FilenoUtil.filenoFrom(readPipe[0]); + int readPipeReadFD = readPipe[0]; eargp.fileActions.add(SpawnFileAction.close(readPipeReadFD)); } if (writePipe != null) { // dup our write pipe's read end into stdin - int writePipeReadFD = FilenoUtil.filenoFrom(writePipe[0]); + int writePipeReadFD = writePipe[0]; eargp.fd_dup2 = checkExecRedirect1(runtime, eargp.fd_dup2, runtime.newFixnum(0), runtime.newFixnum(writePipeReadFD)); // close the other end of the pipe in the child - int writePipeWriteFD = FilenoUtil.filenoFrom(writePipe[1]); + int writePipeWriteFD = writePipe[1]; eargp.fileActions.add(SpawnFileAction.close(writePipeWriteFD)); } } @@ -880,9 +895,21 @@ static int run_exec_dup2(Ruby runtime, RubyArray ary, ExecArg eargp, ExecArg sar static int redirectDup(Ruby runtime, int oldfd) { + // Partial impl of rb_cloexec_fcntl_dup int ret; ret = runtime.getPosix().dup(oldfd); -// ttyprintf("dup(%d) => %d\n", oldfd, ret); + int flags = runtime.getPosix().fcntl(ret, Fcntl.F_GETFD); + runtime.getPosix().fcntlInt(ret, Fcntl.F_SETFD, flags | FcntlLibrary.FD_CLOEXEC); + return ret; + } + + static int redirectCloexecDup(Ruby runtime, int oldfd) + { + int ret = redirectDup(runtime, oldfd); + + int flags = runtime.getPosix().fcntl(ret, Fcntl.F_GETFD); + runtime.getPosix().fcntlInt(ret, Fcntl.F_SETFD, flags | FcntlLibrary.FD_CLOEXEC); + return ret; } @@ -911,8 +938,8 @@ static int saveRedirectFd(Ruby runtime, int fd, ExecArg sargp, String[] errmsg) // child, it's not necessary for us to fix up the parent. if (false && sargp != null) { - IRubyObject newary; - int save_fd = redirectDup(runtime, fd); + RubyArray newary; + int save_fd = redirectCloexecDup(runtime, fd); if (save_fd == -1) { if (runtime.getPosix().errno() == Errno.EBADF.intValue()) return 0; @@ -1007,12 +1034,6 @@ int execargRunOptions(ThreadContext context, Ruby runtime, ExecArg eargp, ExecAr } } - obj = eargp.fd_open; - if (obj != null) { - if (run_exec_open(runtime, (RubyArray)obj, eargp, sargp, errmsg) == -1) /* async-signal-safe */ - return -1; - } - obj = eargp.fd_dup2_child; if (obj != null) { if (run_exec_dup2_child(runtime, (RubyArray)obj, eargp, sargp, errmsg) == -1) /* async-signal-safe */ @@ -1085,58 +1106,6 @@ static int run_exec_close(Ruby runtime, RubyArray ary, ExecArg eargp, String[] e return 0; } - /* This function should be async-signal-safe when sargp is NULL. Actually it is. */ - static int run_exec_open(Ruby runtime, RubyArray ary, ExecArg eargp, ExecArg sargp, String[] errmsg) { - int i; - int ret; - - for (i = 0; i < ary.size(); i++) { - RubyArray elt = (RubyArray)ary.eltOk(i); - int fd; - RubyArray param = (RubyArray)elt.eltOk(1); - String path = param.eltOk(0).toString(); - int flags = RubyNumeric.num2int(param.eltOk(1)); - int perm = RubyNumeric.num2int(param.eltOk(2)); - boolean need_close = true; - // This always succeeds because we defer to posix_spawn - elt = (RubyArray)ary.eltOk(i); - fd = RubyNumeric.fix2int(elt.eltOk(0)); - redirectOpen(eargp, fd, path, flags, perm); /* async-signal-safe */ - - // The rest of this logic is for closing the opened file in parent and preserving - // the file descriptors modified. Since we defer to posix_spawn, this is not needed. - -// rb_update_max_fd(fd2); -// while (i < ary.size() && -// elt.eltOk(1) == param) { -// elt = (RubyArray)ary.eltOk(i); -// fd = RubyNumeric.fix2int(elt.eltOk(0)); -// if (fd == fd2) { -// need_close = false; -// } -// else { -// if (saveRedirectFd(runtime, fd, sargp, errmsg) < 0) /* async-signal-safe */ -// return -1; -// ret = redirectDup2(eargp, fd2, fd); /* async-signal-safe */ -// if (ret == -1) { -// if (errmsg != null) errmsg[0] = "dup2"; -// return -1; -// } -//// rb_update_max_fd(fd); -// } -// i++; -// } -// if (need_close) { -// ret = redirectClose(eargp, fd2); /* async-signal-safe */ -// if (ret == -1) { -// if (errmsg != null) errmsg[0] = "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(Ruby runtime, RubyArray ary, ExecArg eargp, ExecArg sargp, String[] errmsg) { long i; @@ -1170,15 +1139,75 @@ static int runExecDup2TmpbufSize(int n) { } static void execargFixup(ThreadContext context, Ruby runtime, ExecArg eargp) { + execargParentStart(context, runtime, eargp); + } + + static void execargParentStart(ThreadContext context, Ruby runtime, ExecArg eargp) { + try { + execargParentStart1(context, runtime, eargp); + } catch (RaiseException re) { + execargParentEnd(runtime, eargp); + throw re; + } + } + + static void execargParentStart1(ThreadContext context, Ruby runtime, ExecArg eargp) { boolean unsetenv_others; RubyArray envopts; - IRubyObject ary; + RubyArray ary; eargp.redirect_fds = checkExecFds(context, runtime, eargp); + ary = eargp.fd_open; + if (ary != null) { + long i; + for (i = 0; i < ary.size(); i++) { + RubyArray elt = ary.eltOk(i); + int fd = RubyNumeric.fix2int(elt.eltOk(0)); + RubyArray param = elt.eltOk(1); + IRubyObject vpath = param.eltOk(0); + int flags = RubyNumeric.num2int(param.eltOk(1)); + int perm = RubyNumeric.num2int(param.eltOk(2)); + RubyArray fd2v = (RubyArray) param.eltOk(3); + int fd2; + if (fd2v == null) { + RubyIO.Sysopen open_data = new RubyIO.Sysopen(); + vpath = RubyFile.get_path(context, vpath); + // TODO +// vpath = rb_str_encode_ospath(vpath); + while (true) { + open_data.fname = vpath.toString(); + open_data.oflags = flags; + open_data.perm = perm; + ChannelFD ret; + open_data.errno = Errno.EINTR; + ret = open_func(runtime, open_data); +// rb_thread_call_without_gvl2(open_func, (void *)&open_data, RUBY_UBF_IO, 0); + if (ret == null) { + if (open_data.errno == Errno.EINTR) { + context.pollThreadEvents(); + continue; + } + runtime.newErrnoFromInt(open_data.errno.intValue(), vpath.toString()); + } + // We're in the fully-native process logic, so this should be a native stream + fd2 = ((NativeSelectableChannel) ret).getFD(); +// rb_update_max_fd(fd2); + param.eltSetOk(3, runtime.newFixnum(fd2)); + context.pollThreadEvents(); + break; + } + } + else { + fd2 = RubyNumeric.num2int(fd2v); + } + execargAddopt(context, runtime, eargp, runtime.newFixnum(fd), runtime.newFixnum(fd2)); + } + } + ary = eargp.fd_dup2; if (ary != null) { - int len = runExecDup2TmpbufSize(((RubyArray)ary).size()); + int len = runExecDup2TmpbufSize(ary.size()); run_exec_dup2_fd_pair[] tmpbuf = new run_exec_dup2_fd_pair[len]; for (int i = 0; i < tmpbuf.length; i++) tmpbuf[i] = new run_exec_dup2_fd_pair(); eargp.dup2_tmpbuf = tmpbuf; @@ -1221,6 +1250,45 @@ static void execargFixup(ThreadContext context, Ruby runtime, ExecArg eargp) { // RB_GC_GUARD(execarg_obj); } + static void execargParentEnd(Ruby runtime, ExecArg eargp) { + int err = runtime.getPosix().errno(); + RubyArray ary; + + ary = eargp.fd_open; + if (ary != null) { + long i; + for (i = 0; i < ary.size(); i++) { + RubyArray elt = ary.eltOk(i); + RubyArray param = elt.eltOk(1); + IRubyObject fd2v; + int fd2; + fd2v = param.eltOk(3); + if (fd2v != null) { + fd2 = RubyNumeric.fix2int(fd2v); + parentRedirectClose(runtime, fd2); + param.eltSetOk(3, runtime.getNil()); + } + } + } + + runtime.getPosix().errno(err); + } + + static ChannelFD open_func(Ruby runtime, RubyIO.Sysopen data) { + ChannelFD ret = parentRedirectOpen(runtime, data); + data.errno = Errno.valueOf(runtime.getPosix().errno()); + return ret; + } + + static ChannelFD parentRedirectOpen(Ruby runtime, RubyIO.Sysopen data) { + return RubyIO.cloexecOpen(runtime, data); + } + + static void parentRedirectClose(Ruby runtime, int fd) { + // close_unless_reserved + if (fd > 2) runtime.getPosix().close(fd); + } + private static void buildEnvp(Ruby runtime, ExecArg eargp, IRubyObject envtbl) { String[] envp_str; List envp_buf; @@ -1623,20 +1691,20 @@ else if (Platform.IS_WINDOWS && fd >= 3 && iskey) { } // MRI: check_exec_redirect1 - static IRubyObject checkExecRedirect1(Ruby runtime, IRubyObject ary, IRubyObject key, IRubyObject param) { + static RubyArray checkExecRedirect1(Ruby runtime, RubyArray ary, IRubyObject key, IRubyObject param) { if (ary == null) { ary = runtime.newArray(); } if (!(key instanceof RubyArray)) { IRubyObject fd = checkExecRedirectFd(runtime, key, !param.isNil()); - ((RubyArray)ary).push(runtime.newArray(fd, param)); + ary.push(runtime.newArray(fd, param)); } else { int i, n=0; for (i = 0 ; i < ((RubyArray)key).size(); i++) { IRubyObject v = ((RubyArray)key).eltOk(i); IRubyObject fd = checkExecRedirectFd(runtime, v, !param.isNil()); - ((RubyArray)ary).push(runtime.newArray(fd, param)); + ary.push(runtime.newArray(fd, param)); n++; } } @@ -1959,10 +2027,10 @@ public static class ExecArg { int umask_mask; int uid; int gid; - IRubyObject fd_dup2; - IRubyObject fd_close; - IRubyObject fd_open; - IRubyObject fd_dup2_child; + RubyArray fd_dup2; + RubyArray fd_close; + RubyArray fd_open; + RubyArray fd_dup2_child; int close_others_maxhint; RubyArray env_modification; /* null or [[k1,v1], ...] */ String chdir_dir; diff --git a/core/src/main/java/org/jruby/util/io/PosixShim.java b/core/src/main/java/org/jruby/util/io/PosixShim.java index fd905b507d5..9f1ebe4c5fe 100644 --- a/core/src/main/java/org/jruby/util/io/PosixShim.java +++ b/core/src/main/java/org/jruby/util/io/PosixShim.java @@ -405,6 +405,17 @@ public int setCloexec(int fd, boolean cloexec) { return ret; } + public int fcntlSetFD(int fd, int flags) { + int ret = posix.fcntlInt(fd, Fcntl.F_SETFD, flags); + if (ret == -1) errno = Errno.valueOf(posix.errno()); + return ret; + } + + public int fcntlGetFD(int fd) { + int ret = posix.fcntl(fd, Fcntl.F_GETFD); + return ret; + } + public Channel open(String cwd, String path, ModeFlags flags, int perm) { if ((path.equals("/dev/null") || path.equalsIgnoreCase("nul")) && Platform.IS_WINDOWS) { path = "NUL:";