Fixes #4348. File.open with open mode a+ do not work (windows or pure…

…-Java mode)
jruby · Dec 9, 2016 · d26784d · d26784d
1 parent e816e5a
commit d26784d
Show file tree

Hide file tree

Showing 2 changed files with 121 additions and 14 deletions.
diff --git a/core/src/main/java/org/jruby/util/AppendModeChannel.java b/core/src/main/java/org/jruby/util/AppendModeChannel.java
@@ -0,0 +1,113 @@
+package org.jruby.util;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.MappedByteBuffer;
+import java.nio.channels.FileChannel;
+import java.nio.channels.FileLock;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.WritableByteChannel;
+
+/**
+ * A Channel wrapper that will seek to the end of the open file on any write()
+ * operations by performing a seek to eof beforehand.  Motivated by a+ mode which requires
+ * RandomAccessFile in pure-Java mode but has no append option ala O_APPEND to open(2).
+ */
+public class AppendModeChannel extends FileChannel {
+    private FileChannel delegate;
+
+    public AppendModeChannel(FileChannel delegate) {
+        this.delegate = delegate;
+    }
+
+    @Override
+    public int read(ByteBuffer dst) throws IOException {
+        return delegate.read(dst);
+    }
+
+    @Override
+    public long read(ByteBuffer[] dsts, int offset, int length) throws IOException {
+        return delegate.read(dsts, offset, length);
+    }
+
+    @Override
+    public int write(ByteBuffer src) throws IOException {
+        delegate.position(size());
+        return delegate.write(src);
+    }
+
+    @Override
+    public long write(ByteBuffer[] srcs, int offset, int length) throws IOException {
+        delegate.position(size());
+        return delegate.write(srcs, offset, length);
+    }
+
+    @Override
+    public long position() throws IOException {
+        return delegate.position();
+    }
+
+    @Override
+    public FileChannel position(long newPosition) throws IOException {
+        return delegate.position(newPosition);
+    }
+
+    @Override
+    public long size() throws IOException {
+        return delegate.size();
+    }
+
+    @Override
+    public FileChannel truncate(long size) throws IOException {
+        return delegate.truncate(size);
+    }
+
+    @Override
+    public void force(boolean metaData) throws IOException {
+        delegate.force(metaData);
+    }
+
+    @Override
+    public long transferTo(long position, long count, WritableByteChannel target) throws IOException {
+        return delegate.transferTo(position, count, target);
+    }
+
+    @Override
+    public long transferFrom(ReadableByteChannel src, long position, long count) throws IOException {
+        return delegate.transferFrom(src, position, count);
+    }
+
+    @Override
+    public int read(ByteBuffer dst, long position) throws IOException {
+        return delegate.read(dst, position);
+    }
+
+    @Override
+    public int write(ByteBuffer src, long position) throws IOException {
+        delegate.position(size());
+        return delegate.write(src, position);
+    }
+
+    @Override
+    public MappedByteBuffer map(MapMode mode, long position, long size) throws IOException {
+        return delegate.map(mode, position, size);
+    }
+
+    @Override
+    public FileLock lock(long position, long size, boolean shared) throws IOException {
+        return delegate.lock(position, size, shared);
+    }
+
+    @Override
+    public FileLock tryLock(long position, long size, boolean shared) throws IOException {
+        return delegate.tryLock(position, size, shared);
+    }
+
+    @Override
+    protected void implCloseChannel() throws IOException {
+        // This will end up locking the same lock twice but will only call a real
+        // implCloseChannel once which is what is supposed to be the contract of
+        // this method.
+        delegate.close();
+    }
+}
diff --git a/core/src/main/java/org/jruby/util/RegularFileResource.java b/core/src/main/java/org/jruby/util/RegularFileResource.java
@@ -1,5 +1,6 @@
 package org.jruby.util;
 
+import java.nio.channels.SeekableByteChannel;
 import jnr.constants.platform.Errno;
 import jnr.constants.platform.Fcntl;
 import jnr.enxio.channels.NativeDeviceChannel;
@@ -229,27 +230,20 @@ public Channel openChannel(ModeFlags flags, int perm) throws ResourceException {
     }
 
     private Channel createChannel(ModeFlags flags) throws ResourceException {
-        FileChannel fileChannel;
-
-        /* Because RandomAccessFile does not provide a way to pass append
-         * mode, we must manually seek if using RAF. FileOutputStream,
-         * however, does properly honor append mode at the lowest levels,
-         * reducing append write costs when we're only doing writes.
-         *
-         * The code here will use a FileOutputStream if we're only writing,
-         * setting isInAppendMode to true to disable our manual seeking.
-         *
-         * RandomAccessFile does not handle append for us, so if we must
-         * also be readable we pass false for isInAppendMode to indicate
-         * we need manual seeking.
-         */
+        SeekableByteChannel fileChannel;
+
         try{
             if (flags.isWritable() && !flags.isReadable()) {
                 FileOutputStream fos = new FileOutputStream(file, flags.isAppendable());
                 fileChannel = fos.getChannel();
             } else {
                 RandomAccessFile raf = new RandomAccessFile(file, flags.toJavaModeString());
                 fileChannel = raf.getChannel();
+
+                // O_APPEND specifies that all writes will always be at the end of the open file
+                // (even if we happened to have seek'd away from the end of the file o_O). RAF
+                // does not have these semantics so we wrap it to support this unusual case.
+                if (flags.isAppendable()) fileChannel = new AppendModeChannel((FileChannel) fileChannel);
             }
         } catch (FileNotFoundException fnfe) {
             // Jave throws FileNotFoundException both if the file doesn't exist or there were