Skip to content

Commit

Permalink
Fix epoll spliceTo file descriptor with offset (netty#9369)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
njhill authored and normanmaurer committed Jul 16, 2019
1 parent cd824e4 commit 1748352
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 10 deletions.
2 changes: 1 addition & 1 deletion transport-native-epoll/src/main/c/netty_epoll_native.c
Original file line number Diff line number Diff line change
Expand Up @@ -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* 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 {
res = splice(fd, p_off_in, fdOut, p_off_out, (size_t) len, SPLICE_F_NONBLOCK | SPLICE_F_MOVE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public final ChannelFuture spliceTo(final FileDescriptor ch, final int offset, f
public final ChannelFuture spliceTo(final FileDescriptor ch, final int offset, final int len,
final ChannelPromise promise) {
checkPositiveOrZero(len, "len");
checkPositiveOrZero(offset, "offser");
checkPositiveOrZero(offset, "offset");
if (config().getEpollMode() != EpollMode.LEVEL_TRIGGERED) {
throw new IllegalStateException("spliceTo() supported only when using " + EpollMode.LEVEL_TRIGGERED);
}
Expand Down Expand Up @@ -977,7 +977,7 @@ public boolean spliceOut() throws Exception {
private final class SpliceFdTask extends SpliceInTask {
private final FileDescriptor fd;
private final ChannelPromise promise;
private final int offset;
private int offset;

SpliceFdTask(FileDescriptor fd, int offset, int len, ChannelPromise promise) {
super(len, promise);
Expand Down Expand Up @@ -1007,6 +1007,7 @@ public boolean spliceIn(RecvByteBufAllocator.Handle handle) {
}
do {
int splicedOut = Native.splice(pipeIn.intValue(), -1, fd.intValue(), offset, splicedIn);
offset += splicedOut;
splicedIn -= splicedOut;
} while (splicedIn > 0);
if (len == 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public void operationComplete(ChannelFuture future) throws Exception {
}
}

@Test
@Test(timeout = 10000)
public void spliceToFile() throws Throwable {
EventLoopGroup group = new EpollEventLoopGroup(1);
File file = File.createTempFile("netty-splice", null);
Expand All @@ -215,7 +215,7 @@ public void spliceToFile() throws Throwable {
i += length;
}

while (sh.future == null || !sh.future.isDone()) {
while (sh.future2 == null || !sh.future2.isDone() || !sh.future.isDone()) {
if (sh.exception.get() != null) {
break;
}
Expand Down Expand Up @@ -291,22 +291,22 @@ public void exceptionCaught(ChannelHandlerContext ctx,
private static class SpliceHandler extends ChannelInboundHandlerAdapter {
private final File file;

volatile Channel channel;
volatile ChannelFuture future;
volatile ChannelFuture future2;
final AtomicReference<Throwable> exception = new AtomicReference<Throwable>();

SpliceHandler(File file) {
this.file = file;
}

@Override
public void channelActive(ChannelHandlerContext ctx)
throws Exception {
channel = ctx.channel();
public void channelActive(ChannelHandlerContext ctx) throws Exception {
final EpollSocketChannel ch = (EpollSocketChannel) ctx.channel();
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
Expand Down

0 comments on commit 1748352

Please sign in to comment.