commit c2f5a7eeb154821ab2b26ac86b91c26b44929c16
author Elie Kheirallah <khei@google.com> Fri May 27 22:43:40 2022 +0000
committer Elie Kheirallah <khei@google.com> Tue May 31 18:45:58 2022 +0000
tree 9690ed44f570e02709006d041d6206d60bcc799a
parent a4b581ad56c4f491183b3d7513a1663e0a9be8d9

Fix for flaky test in binderLibTest

Added thread locks around the update of mKernelStartedThreads
Added hwasan-presubmit to TEST MAPPING

Bug: 233787404
Test: binderLibTest
Change-Id: If8dcd7061c478a3ebbb0414c4fcce2a9bf512563

libs/binder/ProcessState.cpp

diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index 6beab43..7faff47 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -35,14 +35,15 @@
 
 #include <errno.h>
 #include <fcntl.h>
-#include <mutex>
+#include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <unistd.h>
+#include <mutex>
 
 #define BINDER_VM_SIZE ((1 * 1024 * 1024) - sysconf(_SC_PAGE_SIZE) * 2)
 #define DEFAULT_MAX_BINDER_THREADS 15
@@ -399,7 +400,9 @@
         ALOGV("Spawning new pooled thread, name=%s\n", name.string());
         sp<Thread> t = sp<PoolThread>::make(isMain);
         t->run(name.string());
+        pthread_mutex_lock(&mThreadCountLock);
         mKernelStartedThreads++;
+        pthread_mutex_unlock(&mThreadCountLock);
     }
 }
 

libs/binder/TEST_MAPPING

diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING
index ebb0d27..0232f50 100644
--- a/libs/binder/TEST_MAPPING
+++ b/libs/binder/TEST_MAPPING
@@ -83,5 +83,10 @@
     {
       "name": "rustBinderSerializationTest"
     }
+  ],
+ "hwasan-presubmit": [
+    {
+      "name": "binderLibTest"
+    }
   ]
 }

libs/binder/tests/binderLibTest.cpp

diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 18a9f86..3e90726 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -1280,7 +1280,7 @@
                 StatusEq(NO_ERROR));
     replyi = reply.readInt32();
     // No more than 16 threads should exist.
-    EXPECT_EQ(replyi, kKernelThreads + 1);
+    EXPECT_TRUE(replyi == kKernelThreads || replyi == kKernelThreads + 1);
 }
 
 size_t epochMillis() {
@@ -1726,11 +1726,11 @@
                 return NO_ERROR;
             }
             case BINDER_LIB_TEST_PROCESS_LOCK: {
-                blockMutex.lock();
+                m_blockMutex.lock();
                 return NO_ERROR;
             }
             case BINDER_LIB_TEST_LOCK_UNLOCK: {
-                std::lock_guard<std::mutex> _l(blockMutex);
+                std::lock_guard<std::mutex> _l(m_blockMutex);
                 return NO_ERROR;
             }
             case BINDER_LIB_TEST_UNLOCK_AFTER_MS: {
@@ -1738,10 +1738,11 @@
                 return unlockInMs(ms);
             }
             case BINDER_LIB_TEST_PROCESS_TEMPORARY_LOCK: {
-                blockMutex.lock();
-                std::thread t([&] {
-                    unlockInMs(data.readInt32());
-                }); // start local thread to unlock in 1s
+                m_blockMutex.lock();
+                sp<BinderLibTestService> thisService = this;
+                int32_t value = data.readInt32();
+                // start local thread to unlock in 1s
+                std::thread t([=] { thisService->unlockInMs(value); });
                 t.detach();
                 return NO_ERROR;
             }
@@ -1752,7 +1753,7 @@
 
     status_t unlockInMs(int32_t ms) {
         usleep(ms * 1000);
-        blockMutex.unlock();
+        m_blockMutex.unlock();
         return NO_ERROR;
     }
 
@@ -1766,7 +1767,7 @@
     sp<IBinder> m_strongRef;
     sp<IBinder> m_callback;
     bool m_exitOnDestroy;
-    std::mutex blockMutex;
+    std::mutex m_blockMutex;
 };
 
 int run_server(int index, int readypipefd, bool usePoll)