From 08bdb15d748d7d9fb6a01894fe9034699a01b764 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
(cherry picked from commit a7ec8300d98e92c1363303f7f103bf0787d13a27)
---
 .../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 12fe625370..075fce69a0 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
@@ -476,7 +476,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(
@@ -506,7 +510,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);
         }
@@ -514,10 +520,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);
         }