Skip to content

Commit

Permalink
Mutiple fixes for pipes and subprocess IO.
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@headius
headius committed Dec 1, 2017
1 parent d8dc9f3 commit ac40a48
Show file tree
Hide file tree
Showing 6 changed files with 292 additions and 131 deletions.
45 changes: 23 additions & 22 deletions core/src/main/java/org/jruby/RubyIO.java
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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. */
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
}
Expand Down
58 changes: 58 additions & 0 deletions core/src/main/java/org/jruby/api/API.java
View file
@@ -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);
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/util/io/ChannelFD.java
View file
Expand Up @@ -84,7 +84,7 @@ public int dup2From(POSIX posix, ChannelFD dup2Source) {

this.currentLock = dup2Source.currentLock;

return 0;
return fakeFileno;
}

public void close() throws IOException {
Expand Down
27 changes: 25 additions & 2 deletions core/src/main/java/org/jruby/util/io/OpenFile.java
View file
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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.
*
Expand Down

0 comments on commit ac40a48

Please sign in to comment.