From 36529b250a74a93e3040dc0d94d33951e380c61c Mon Sep 17 00:00:00 2001
From: Alan Stokes <alanstokes@google.com>
Date: Fri, 6 Nov 2020 16:57:53 +0000
Subject: [PATCH] Fix race condition in NetworkWatchListTest

We were running cp to create a file and then running a command to read
the file. However were weren't waiting for the cp to finish (or,
indeed, start), so occasionally the read would fail saying the file
didn't exist.

Also added some logging, because diagnosing failure without it is
painful. Also simplify & improve file closing logic.

Some of this was previously submitted on internal master as change
I1c875102f0cce32cbbe2e3b36de913741c9abb92, but I've reverted that in
favor of this.

Bug: 168216494
Test: atest CtsNetTestCases:android.net.cts.NetworkWatchlistTest
Change-Id: I46a9db8b7a1885a9829f00bbd3233b863cfd1f5d
---
 .../android/net/cts/NetworkWatchlistTest.java | 81 ++++++++++---------
 1 file changed, 45 insertions(+), 36 deletions(-)

diff --git a/tests/cts/net/src/android/net/cts/NetworkWatchlistTest.java b/tests/cts/net/src/android/net/cts/NetworkWatchlistTest.java
index 81a9e30dd5..6833c70994 100644
--- a/tests/cts/net/src/android/net/cts/NetworkWatchlistTest.java
+++ b/tests/cts/net/src/android/net/cts/NetworkWatchlistTest.java
@@ -22,11 +22,13 @@ import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assume.assumeTrue;
 
+import android.app.UiAutomation;
 import android.content.Context;
 import android.net.ConnectivityManager;
-import android.platform.test.annotations.AppModeFull;
 import android.os.FileUtils;
 import android.os.ParcelFileDescriptor;
+import android.platform.test.annotations.AppModeFull;
+import android.util.Log;
 
 import androidx.test.InstrumentationRegistry;
 import androidx.test.filters.SmallTest;
@@ -117,42 +119,9 @@ public class NetworkWatchlistTest {
         return formatter.toString();
     }
 
-    private void saveResourceToFile(String res, String filePath) throws IOException {
-        // App can't access /data/local/tmp directly, so we pipe resource to file through stdin.
-        ParcelFileDescriptor stdin = pipeFromStdin(filePath);
-        pipeResourceToFileDescriptor(res, stdin);
-    }
-
-    /* Pipe stdin to a file in filePath. Returns PFD for stdin. */
-    private ParcelFileDescriptor pipeFromStdin(String filePath) {
-        // Not all devices have symlink for /dev/stdin, so use /proc/self/fd/0 directly.
-        // /dev/stdin maps to /proc/self/fd/0.
-        return runRwCommand("cp /proc/self/fd/0 " + filePath)[1];
-    }
-
-    private void pipeResourceToFileDescriptor(String res, ParcelFileDescriptor pfd)
-            throws IOException {
-        InputStream resStream = getClass().getClassLoader().getResourceAsStream(res);
-        FileOutputStream fdStream = new ParcelFileDescriptor.AutoCloseOutputStream(pfd);
-
-        FileUtils.copy(resStream, fdStream);
-
-        try {
-            fdStream.close();
-        } catch (IOException e) {
-        }
-    }
-
-    private static String runCommand(String command) throws IOException {
-        return SystemUtil.runShellCommand(InstrumentationRegistry.getInstrumentation(), command);
-    }
-
-    private static ParcelFileDescriptor[] runRwCommand(String command) {
-        return InstrumentationRegistry.getInstrumentation()
-                .getUiAutomation().executeShellCommandRw(command);
-    }
-
     private void setWatchlistConfig(String watchlistConfigFile) throws Exception {
+        Log.w("NetworkWatchlistTest", "Setting watchlist config " + watchlistConfigFile
+                + " in " + Thread.currentThread().getName());
         cleanup();
         saveResourceToFile(watchlistConfigFile, TMP_CONFIG_PATH);
         final String cmdResult = runCommand(
@@ -160,4 +129,44 @@ public class NetworkWatchlistTest {
         assertThat(cmdResult).contains("Success");
         cleanup();
     }
+
+    private void saveResourceToFile(String res, String filePath) throws IOException {
+        final UiAutomation uiAutomation = InstrumentationRegistry.getInstrumentation()
+                .getUiAutomation();
+        // App can't access /data/local/tmp directly, so we pipe resource to file through stdin.
+        // Not all devices have symlink for /dev/stdin, so use /proc/self/fd/0 directly.
+        // /dev/stdin maps to /proc/self/fd/0.
+        final ParcelFileDescriptor[] fileDescriptors = uiAutomation.executeShellCommandRw(
+                "cp /proc/self/fd/0 " + filePath);
+
+        ParcelFileDescriptor stdin = fileDescriptors[1];
+        ParcelFileDescriptor stdout = fileDescriptors[0];
+
+        pipeResourceToFileDescriptor(res, stdin);
+
+        // Wait for the process to close its stdout - which should mean it has completed.
+        consumeFile(stdout);
+    }
+
+    private void consumeFile(ParcelFileDescriptor pfd) throws IOException {
+        try (InputStream stream = new ParcelFileDescriptor.AutoCloseInputStream(pfd)) {
+            for (;;) {
+                if (stream.read() == -1) {
+                    return;
+                }
+            }
+        }
+    }
+
+    private void pipeResourceToFileDescriptor(String res, ParcelFileDescriptor pfd)
+            throws IOException {
+        try (InputStream resStream = getClass().getClassLoader().getResourceAsStream(res);
+                FileOutputStream fdStream = new ParcelFileDescriptor.AutoCloseOutputStream(pfd)) {
+            FileUtils.copy(resStream, fdStream);
+        }
+    }
+
+    private static String runCommand(String command) throws IOException {
+        return SystemUtil.runShellCommand(InstrumentationRegistry.getInstrumentation(), command);
+    }
 }