Handle recvfrom() returning 0.
According to the Linux man page, recvfrom() can return 0 when the peer has performed an orderly shutdown. This also happens when other thread uses a shutdown function on a socket file descriptor.
libcore.io.Posix#recvfrom was treating this case as a success and
was trying to interpret unpopulated sockaddr structure, throwing
IllegalArgumentException in result.
DatagramChannel#receive in same case would return
last previously seen SocketAddress.
This change makes sure that libcore.io.Posix#recvfrom and
java.nio.channels.DatagramChannel handle this case correctly.
Bug: 27294715
Bug: 27233089
Change-Id: I1ec58327efbe9ea6b8d36716756a02987e3b9897
diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/java/nio/channels/DatagramChannelTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/java/nio/channels/DatagramChannelTest.java
index 12aaae0..6bfe85e 100644
--- a/harmony-tests/src/test/java/org/apache/harmony/tests/java/nio/channels/DatagramChannelTest.java
+++ b/harmony-tests/src/test/java/org/apache/harmony/tests/java/nio/channels/DatagramChannelTest.java
@@ -17,6 +17,8 @@
package org.apache.harmony.tests.java.nio.channels;
+import android.system.ErrnoException;
+import android.system.OsConstants;
import java.io.IOException;
import java.net.DatagramPacket;
import java.net.DatagramSocket;
@@ -33,8 +35,10 @@
import java.nio.channels.UnresolvedAddressException;
import java.nio.channels.UnsupportedAddressTypeException;
import java.nio.channels.spi.SelectorProvider;
+import java.util.concurrent.atomic.AtomicReference;
import junit.framework.TestCase;
import libcore.io.IoUtils;
+import libcore.io.Libcore;
/**
* Test for DatagramChannel
@@ -2518,4 +2522,40 @@
dc.socket().getLocalSocketAddress();
}
+
+ // b/27294715
+ public void test_concurrentShutdown() throws Exception {
+ DatagramChannel dc = DatagramChannel.open();
+ dc.configureBlocking(true);
+ dc.bind(new InetSocketAddress(Inet6Address.LOOPBACK, 0));
+ // Set 4s timeout
+ dc.socket().setSoTimeout(4000);
+
+ final AtomicReference<Exception> killerThreadException = new AtomicReference<Exception>(null);
+ final Thread killer = new Thread(new Runnable() {
+ public void run() {
+ try {
+ Thread.sleep(2000);
+ try {
+ Libcore.os.shutdown(dc.socket().getFileDescriptor$(), OsConstants.SHUT_RDWR);
+ } catch (ErrnoException expected) {
+ if (OsConstants.ENOTCONN != expected.errno) {
+ killerThreadException.set(expected);
+ }
+ }
+ } catch (Exception ex) {
+ killerThreadException.set(ex);
+ }
+ }
+ });
+ killer.start();
+
+ ByteBuffer dst = ByteBuffer.allocate(CAPACITY_NORMAL);
+ assertEquals(null, dc.receive(dst));
+ assertEquals(0, dst.position());
+ dc.close();
+
+ killer.join();
+ assertNull(killerThreadException.get());
+ }
}
diff --git a/luni/src/main/native/libcore_io_Posix.cpp b/luni/src/main/native/libcore_io_Posix.cpp
index 27fa0d7..3fa4dee 100644
--- a/luni/src/main/native/libcore_io_Posix.cpp
+++ b/luni/src/main/native/libcore_io_Posix.cpp
@@ -376,8 +376,8 @@
const struct sockaddr_nl* nl_addr = reinterpret_cast<const struct sockaddr_nl*>(&ss);
static jmethodID ctor = env->GetMethodID(JniConstants::netlinkSocketAddressClass,
"<init>", "(II)V");
- return env->NewObject(JniConstants::netlinkSocketAddressClass, ctor,
- static_cast<jint>(nl_addr->nl_pid),
+ return env->NewObject(JniConstants::netlinkSocketAddressClass, ctor,
+ static_cast<jint>(nl_addr->nl_pid),
static_cast<jint>(nl_addr->nl_groups));
} else if (ss.ss_family == AF_PACKET) {
const struct sockaddr_ll* sll = reinterpret_cast<const struct sockaddr_ll*>(&ss);
@@ -1516,10 +1516,9 @@
sockaddr* from = (javaInetSocketAddress != NULL) ? reinterpret_cast<sockaddr*>(&ss) : NULL;
socklen_t* fromLength = (javaInetSocketAddress != NULL) ? &sl : 0;
jint recvCount = NET_FAILURE_RETRY(env, ssize_t, recvfrom, javaFd, bytes.get() + byteOffset, byteCount, flags, from, fromLength);
- if (recvCount == -1) {
- return recvCount;
+ if (recvCount > 0) {
+ fillInetSocketAddress(env, javaInetSocketAddress, ss);
}
- fillInetSocketAddress(env, javaInetSocketAddress, ss);
return recvCount;
}
diff --git a/luni/src/test/java/libcore/io/OsTest.java b/luni/src/test/java/libcore/io/OsTest.java
index 552c2a1..49a592d 100644
--- a/luni/src/test/java/libcore/io/OsTest.java
+++ b/luni/src/test/java/libcore/io/OsTest.java
@@ -34,10 +34,12 @@
import java.net.InetSocketAddress;
import java.net.NetworkInterface;
import java.net.ServerSocket;
+import java.net.SocketOptions;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Locale;
+import java.util.concurrent.atomic.AtomicReference;
import junit.framework.TestCase;
import static android.system.OsConstants.*;
@@ -457,6 +459,42 @@
}
}
+ // b/27294715
+ public void test_recvfrom_concurrentShutdown() throws Exception {
+ final FileDescriptor serverFd = Libcore.os.socket(AF_INET, SOCK_DGRAM, 0);
+ Libcore.os.bind(serverFd, InetAddress.getByName("127.0.0.1"), 0);
+ // Set 4s timeout
+ IoBridge.setSocketOption(serverFd, SocketOptions.SO_TIMEOUT, new Integer(4000));
+
+ final AtomicReference<Exception> killerThreadException = new AtomicReference<Exception>(null);
+ final Thread killer = new Thread(new Runnable() {
+ public void run() {
+ try {
+ Thread.sleep(2000);
+ try {
+ Libcore.os.shutdown(serverFd, SHUT_RDWR);
+ } catch (ErrnoException expected) {
+ if (OsConstants.ENOTCONN != expected.errno) {
+ killerThreadException.set(expected);
+ }
+ }
+ } catch (Exception ex) {
+ killerThreadException.set(ex);
+ }
+ }
+ });
+ killer.start();
+
+ ByteBuffer buffer = ByteBuffer.allocate(16);
+ InetSocketAddress srcAddress = new InetSocketAddress();
+ int received = Libcore.os.recvfrom(serverFd, buffer, 0, srcAddress);
+ assertTrue(received == 0);
+ Libcore.os.close(serverFd);
+
+ killer.join();
+ assertNull(killerThreadException.get());
+ }
+
public void test_xattr() throws Exception {
final String NAME_TEST = "user.meow";
diff --git a/ojluni/src/main/native/DatagramChannelImpl.c b/ojluni/src/main/native/DatagramChannelImpl.c
index 473a98d..471c62b 100644
--- a/ojluni/src/main/native/DatagramChannelImpl.c
+++ b/ojluni/src/main/native/DatagramChannelImpl.c
@@ -169,6 +169,15 @@
}
} while (retry == JNI_TRUE);
+ // Peer (or other thread) has performed an orderly shutdown, sockaddr will be
+ // invalid.
+ if (n == 0) {
+ // zero the sender field, so receive() returns null and not
+ // random garbage
+ (*env)->SetObjectField(env, this, dci_senderID, NULL);
+ return n;
+ }
+
/*
* If the source address and port match the cached address
* and port in DatagramChannelImpl then we don't need to
