From f0d45503bc402c8dbfaa866c652d901c1bfd115a Mon Sep 17 00:00:00 2001
From: Lorenzo Colitti <lorenzo@google.com>
Date: Thu, 4 Apr 2019 09:18:29 -0700
Subject: [PATCH] Fix flaky UdpEncapsulationSocket test

This commit reduces the flakiness of the
testOpenAndCloseUdpEncapsulationSocket by retrying up to three times.
Unfortunately, testing port-selected socket creation is racy against
other applications. This helps to handle the same race condition as done
in IpSecService#bindToRandomPort

Bug: 128024100
Test: 200x runs of testOpenAndCloseUdpEncapsulationSocket
Change-Id: I7e036ce821019dbac6c50899bd0894e89d2fe82a
Merged-In: Idf040a67e53d9b9ec6e6c647ce24f8ada501d355
Merged-In: Iad9aea4b42cd8b31a5a2659bb9cb54dd1c64e8b7
(cherry picked from commit 614ab3dd4e49e9b664f5065983fb9067148fef12)
---
 .../core/java/com/android/server/IpSecService.java |  3 +-
 .../java/com/android/server/IpSecServiceTest.java  | 34 +++++++++++++---------
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java
index 126bf6556538..2cfcecca5f99 100644
--- a/services/core/java/com/android/server/IpSecService.java
+++ b/services/core/java/com/android/server/IpSecService.java
@@ -96,9 +96,10 @@ public class IpSecService extends IIpSecService.Stub {
             new int[] {OsConstants.AF_INET, OsConstants.AF_INET6};
 
     private static final int NETD_FETCH_TIMEOUT_MS = 5000; // ms
-    private static final int MAX_PORT_BIND_ATTEMPTS = 10;
     private static final InetAddress INADDR_ANY;
 
+    @VisibleForTesting static final int MAX_PORT_BIND_ATTEMPTS = 10;
+
     static {
         try {
             INADDR_ANY = InetAddress.getByAddress(new byte[] {0, 0, 0, 0});
diff --git a/tests/net/java/com/android/server/IpSecServiceTest.java b/tests/net/java/com/android/server/IpSecServiceTest.java
index b5c3e9287467..4a35015044ff 100644
--- a/tests/net/java/com/android/server/IpSecServiceTest.java
+++ b/tests/net/java/com/android/server/IpSecServiceTest.java
@@ -156,10 +156,21 @@ public class IpSecServiceTest {
 
     @Test
     public void testOpenAndCloseUdpEncapsulationSocket() throws Exception {
-        int localport = findUnusedPort();
+        int localport = -1;
+        IpSecUdpEncapResponse udpEncapResp = null;
+
+        for (int i = 0; i < IpSecService.MAX_PORT_BIND_ATTEMPTS; i++) {
+            localport = findUnusedPort();
+
+            udpEncapResp = mIpSecService.openUdpEncapsulationSocket(localport, new Binder());
+            assertNotNull(udpEncapResp);
+            if (udpEncapResp.status == IpSecManager.Status.OK) {
+                break;
+            }
+
+            // Else retry to reduce possibility for port-bind failures.
+        }
 
-        IpSecUdpEncapResponse udpEncapResp =
-                mIpSecService.openUdpEncapsulationSocket(localport, new Binder());
         assertNotNull(udpEncapResp);
         assertEquals(IpSecManager.Status.OK, udpEncapResp.status);
         assertEquals(localport, udpEncapResp.port);
@@ -204,12 +215,11 @@ public class IpSecServiceTest {
 
     @Test
     public void testOpenUdpEncapsulationSocketAfterClose() throws Exception {
-        int localport = findUnusedPort();
         IpSecUdpEncapResponse udpEncapResp =
-                mIpSecService.openUdpEncapsulationSocket(localport, new Binder());
+                mIpSecService.openUdpEncapsulationSocket(0, new Binder());
         assertNotNull(udpEncapResp);
         assertEquals(IpSecManager.Status.OK, udpEncapResp.status);
-        assertEquals(localport, udpEncapResp.port);
+        int localport = udpEncapResp.port;
 
         mIpSecService.closeUdpEncapsulationSocket(udpEncapResp.resourceId);
         udpEncapResp.fileDescriptor.close();
@@ -226,12 +236,11 @@ public class IpSecServiceTest {
      */
     @Test
     public void testUdpEncapPortNotReleased() throws Exception {
-        int localport = findUnusedPort();
         IpSecUdpEncapResponse udpEncapResp =
-                mIpSecService.openUdpEncapsulationSocket(localport, new Binder());
+                mIpSecService.openUdpEncapsulationSocket(0, new Binder());
         assertNotNull(udpEncapResp);
         assertEquals(IpSecManager.Status.OK, udpEncapResp.status);
-        assertEquals(localport, udpEncapResp.port);
+        int localport = udpEncapResp.port;
 
         udpEncapResp.fileDescriptor.close();
 
@@ -273,14 +282,11 @@ public class IpSecServiceTest {
 
     @Test
     public void testOpenUdpEncapsulationSocketTwice() throws Exception {
-        int localport = findUnusedPort();
-
         IpSecUdpEncapResponse udpEncapResp =
-                mIpSecService.openUdpEncapsulationSocket(localport, new Binder());
+                mIpSecService.openUdpEncapsulationSocket(0, new Binder());
         assertNotNull(udpEncapResp);
         assertEquals(IpSecManager.Status.OK, udpEncapResp.status);
-        assertEquals(localport, udpEncapResp.port);
-        mIpSecService.openUdpEncapsulationSocket(localport, new Binder());
+        int localport = udpEncapResp.port;
 
         IpSecUdpEncapResponse testUdpEncapResp =
                 mIpSecService.openUdpEncapsulationSocket(localport, new Binder());
-- 
2.11.0