Fix epoll spliceTo file descriptor with offset (#9369)

Motivation

The AbstractEpollStreamChannel::spliceTo(FileDescriptor, ...) methods
take an offset parameter but this was effectively ignored due to what
looks like a typo in the corresponding JNI function impl. Instead it
would always use the file's own native offset.

Modification

- Fix typo in netty_epoll_native_splice0() and offset accounting in
AbstractEpollStreamChannel::SpliceFdTask.
- Modify unit test to include an invocation of the public spliceTo
method using non-zero offset.

Result

spliceTo FD methods work as expected when an offset is provided.
This commit is contained in:
Nick Hill 2019-07-16 04:22:30 -07:00 committed by Norman Maurer
parent ef24732640
commit afbbc11c8f
3 changed files with 11 additions and 10 deletions

View File

@ -385,7 +385,7 @@ static jint netty_epoll_native_splice0(JNIEnv* env, jclass clazz, jint fd, jlong
loff_t off_out = (loff_t) offOut; loff_t off_out = (loff_t) offOut;
loff_t* p_off_in = off_in >= 0 ? &off_in : NULL; loff_t* p_off_in = off_in >= 0 ? &off_in : NULL;
loff_t* p_off_out = off_in >= 0 ? &off_out : NULL; loff_t* p_off_out = off_out >= 0 ? &off_out : NULL;
do { do {
res = splice(fd, p_off_in, fdOut, p_off_out, (size_t) len, SPLICE_F_NONBLOCK | SPLICE_F_MOVE); res = splice(fd, p_off_in, fdOut, p_off_out, (size_t) len, SPLICE_F_NONBLOCK | SPLICE_F_MOVE);

View File

@ -201,7 +201,7 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel im
public final ChannelFuture spliceTo(final FileDescriptor ch, final int offset, final int len, public final ChannelFuture spliceTo(final FileDescriptor ch, final int offset, final int len,
final ChannelPromise promise) { final ChannelPromise promise) {
checkPositiveOrZero(len, "len"); checkPositiveOrZero(len, "len");
checkPositiveOrZero(offset, "offser"); checkPositiveOrZero(offset, "offset");
if (config().getEpollMode() != EpollMode.LEVEL_TRIGGERED) { if (config().getEpollMode() != EpollMode.LEVEL_TRIGGERED) {
throw new IllegalStateException("spliceTo() supported only when using " + EpollMode.LEVEL_TRIGGERED); throw new IllegalStateException("spliceTo() supported only when using " + EpollMode.LEVEL_TRIGGERED);
} }
@ -949,7 +949,7 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel im
private final class SpliceFdTask extends SpliceInTask { private final class SpliceFdTask extends SpliceInTask {
private final FileDescriptor fd; private final FileDescriptor fd;
private final ChannelPromise promise; private final ChannelPromise promise;
private final int offset; private int offset;
SpliceFdTask(FileDescriptor fd, int offset, int len, ChannelPromise promise) { SpliceFdTask(FileDescriptor fd, int offset, int len, ChannelPromise promise) {
super(len, promise); super(len, promise);
@ -979,6 +979,7 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel im
} }
do { do {
int splicedOut = Native.splice(pipeIn.intValue(), -1, fd.intValue(), offset, splicedIn); int splicedOut = Native.splice(pipeIn.intValue(), -1, fd.intValue(), offset, splicedIn);
offset += splicedOut;
splicedIn -= splicedOut; splicedIn -= splicedOut;
} while (splicedIn > 0); } while (splicedIn > 0);
if (len == 0) { if (len == 0) {

View File

@ -179,7 +179,7 @@ public class EpollSpliceTest {
} }
} }
@Test @Test(timeout = 10000)
public void spliceToFile() throws Throwable { public void spliceToFile() throws Throwable {
EventLoopGroup group = new MultithreadEventLoopGroup(1, EpollHandler.newFactory()); EventLoopGroup group = new MultithreadEventLoopGroup(1, EpollHandler.newFactory());
File file = File.createTempFile("netty-splice", null); File file = File.createTempFile("netty-splice", null);
@ -205,7 +205,7 @@ public class EpollSpliceTest {
i += length; i += length;
} }
while (sh.future == null || !sh.future.isDone()) { while (sh.future2 == null || !sh.future2.isDone() || !sh.future.isDone()) {
if (sh.exception.get() != null) { if (sh.exception.get() != null) {
break; break;
} }
@ -281,8 +281,8 @@ public class EpollSpliceTest {
private static class SpliceHandler implements ChannelHandler { private static class SpliceHandler implements ChannelHandler {
private final File file; private final File file;
volatile Channel channel;
volatile ChannelFuture future; volatile ChannelFuture future;
volatile ChannelFuture future2;
final AtomicReference<Throwable> exception = new AtomicReference<>(); final AtomicReference<Throwable> exception = new AtomicReference<>();
SpliceHandler(File file) { SpliceHandler(File file) {
@ -290,13 +290,13 @@ public class EpollSpliceTest {
} }
@Override @Override
public void channelActive(ChannelHandlerContext ctx) public void channelActive(ChannelHandlerContext ctx) throws Exception {
throws Exception {
channel = ctx.channel();
final EpollSocketChannel ch = (EpollSocketChannel) ctx.channel(); final EpollSocketChannel ch = (EpollSocketChannel) ctx.channel();
final FileDescriptor fd = FileDescriptor.from(file); final FileDescriptor fd = FileDescriptor.from(file);
future = ch.spliceTo(fd, 0, data.length); // splice two halves separately to test starting offset
future = ch.spliceTo(fd, 0, data.length / 2);
future2 = ch.spliceTo(fd, data.length / 2, data.length / 2);
} }
@Override @Override