From a7ec8300d98e92c1363303f7f103bf0787d13a27 Mon Sep 17 00:00:00 2001
From: Lorenzo Colitti <lorenzo@google.com>
Date: Tue, 7 Mar 2017 00:22:33 +0900
Subject: [PATCH] Dup fds to stop finalizers from invalidating them.

The hostside VPN tests were failing because finalizers were
closing the ParcelFileDescriptors that we use to get socekt fds.
The close operations were causing the fds to be marked as invalid
(i.e., -1), causing the tests' system calls on them to fail with
EBADF.

Fix this by dup(2)ing the fds and closing the original objects.

Also, add some asserts to debug this sort of failure.

Fix: 35927643
Test: HostsideVpnTests passed 20/20 times on bullhead
Change-Id: If88530b5bd32622bd4726cd6f0907f731209bb43
---
 .../net/hostside/RemoteSocketFactoryClient.java   | 15 ++++++++++++---
 .../src/com/android/cts/net/hostside/VpnTest.java | 10 +++++++++-
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/tests/cts/hostside/app/src/com/android/cts/net/hostside/RemoteSocketFactoryClient.java b/tests/cts/hostside/app/src/com/android/cts/net/hostside/RemoteSocketFactoryClient.java
index 799fe50ebe..80f99b6605 100644
--- a/tests/cts/hostside/app/src/com/android/cts/net/hostside/RemoteSocketFactoryClient.java
+++ b/tests/cts/hostside/app/src/com/android/cts/net/hostside/RemoteSocketFactoryClient.java
@@ -22,11 +22,15 @@ import android.content.Intent;
 import android.content.ServiceConnection;
 import android.os.ConditionVariable;
 import android.os.IBinder;
+import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
+import android.system.ErrnoException;
+import android.system.Os;
 
 import com.android.cts.net.hostside.IRemoteSocketFactory;
 
 import java.io.FileDescriptor;
+import java.io.IOException;
 
 public class RemoteSocketFactoryClient {
     private static final int TIMEOUT_MS = 5000;
@@ -76,9 +80,14 @@ public class RemoteSocketFactoryClient {
         }
     }
 
-    public FileDescriptor openSocketFd(
-            String host, int port, int timeoutMs) throws RemoteException {
-        return mService.openSocketFd(host, port, timeoutMs).getFileDescriptor();
+    public FileDescriptor openSocketFd(String host, int port, int timeoutMs)
+            throws RemoteException, ErrnoException, IOException {
+        // Dup the filedescriptor so ParcelFileDescriptor's finalizer doesn't garbage collect it
+        // and cause our fd to become invalid. http://b/35927643 .
+        ParcelFileDescriptor pfd = mService.openSocketFd(host, port, timeoutMs);
+        FileDescriptor fd = Os.dup(pfd.getFileDescriptor());
+        pfd.close();
+        return fd;
     }
 
     public String getPackageName() throws RemoteException {
diff --git a/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java b/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java
index a8ad2b868e..2bd3c39d2c 100755
--- a/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java
+++ b/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java
@@ -481,7 +481,11 @@ public class VpnTest extends InstrumentationTestCase {
     private FileDescriptor openSocketFd(String host, int port, int timeoutMs) throws Exception {
         Socket s = new Socket(host, port);
         s.setSoTimeout(timeoutMs);
-        return ParcelFileDescriptor.fromSocket(s).getFileDescriptor();
+        // Dup the filedescriptor so ParcelFileDescriptor's finalizer doesn't garbage collect it
+        // and cause our fd to become invalid. http://b/35927643 .
+        FileDescriptor fd = Os.dup(ParcelFileDescriptor.fromSocket(s).getFileDescriptor());
+        s.close();
+        return fd;
     }
 
     private FileDescriptor openSocketFdInOtherApp(
@@ -511,7 +515,9 @@ public class VpnTest extends InstrumentationTestCase {
 
     private void assertSocketStillOpen(FileDescriptor fd, String host) throws Exception {
         try {
+            assertTrue(fd.valid());
             sendRequest(fd, host);
+            assertTrue(fd.valid());
         } finally {
             Os.close(fd);
         }
@@ -519,10 +525,12 @@ public class VpnTest extends InstrumentationTestCase {
 
     private void assertSocketClosed(FileDescriptor fd, String host) throws Exception {
         try {
+            assertTrue(fd.valid());
             sendRequest(fd, host);
             fail("Socket opened before VPN connects should be closed when VPN connects");
         } catch (ErrnoException expected) {
             assertEquals(ECONNABORTED, expected.errno);
+            assertTrue(fd.valid());
         } finally {
             Os.close(fd);
         }