commit 2a42935f0b273904cb684d9f6f563d79576ff151
author Ken Chen <cken@google.com> Sat Dec 22 21:46:55 2018 +0800
committer Ken Chen <cken@google.com> Thu Feb 14 11:48:21 2019 +0800
tree 2afe522d290cee405d7b07ae7ecf403d5eacdc60
parent 4fe1b9c0640d079288fe8642869e89290cc368d4

Improve resolver cache lock and thread synchronization

This change comprises:
1. replace pthread_mutex with std::mutex to realize RAII.
2. have a single condition variable to prevent race condition between
   threads.
3. add 'predicate' to avoid spurious awakenings.
4. add a parameter in GetResolverInfo API which enabling test cases to
   know if timeout happened among concurrent DNS queries. Also, dump it in
   bugreport.
5. verify if test case GetAddrInfoV6_concurrent,
   GetAddrInfoStressTest_Binder_100 and
   GetAddrInfoStressTest_Binder_100000 pass because of timeout on
   concurrent DNS queries.

Bug: 120794954
Test: runtests.sh pass
Test: run ResolverTest.GetAddrInfoV6_concurrent 100 times
Test: run ResolverTest.GetAddrInfoStressTest_Binder_100 100 times
Test: run ResolverTest.GetAddrInfoStressTest_Binder_100000 100 times
The test script:
----------------------------
for i in $(seq 1 100)
do
  adb shell resolv_integration_test --gtest_filter=ResolverTest.GetAddrInfoV6_concurrent
  adb shell resolv_integration_test --gtest_filter=ResolverTest.GetAddrInfoStressTest_Binder_100
  adb shell resolv_integration_test --gtest_filter=ResolverTest.GetAddrInfoStressTest_Binder_100000
done
exit 0

Change-Id: I4bdc394ba7ded7a6b7239f2d35b559a4262cb7b9

server/ResolverController.cpp

diff --git a/server/ResolverController.cpp b/server/ResolverController.cpp
index 340af6a..8ff4282 100644
--- a/server/ResolverController.cpp
+++ b/server/ResolverController.cpp
@@ -156,8 +156,9 @@
 }
 
 int ResolverController::getDnsInfo(unsigned netId, std::vector<std::string>* servers,
-        std::vector<std::string>* domains, __res_params* params,
-        std::vector<android::net::ResolverStats>* stats) {
+                                   std::vector<std::string>* domains, __res_params* params,
+                                   std::vector<android::net::ResolverStats>* stats,
+                                   std::vector<int32_t>* wait_for_pending_req_timeout_count) {
     using android::net::ResolverStats;
     using android::net::INetd;
     static_assert(ResolverStats::STATS_SUCCESSES == INetd::RESOLVER_STATS_SUCCESSES &&
@@ -178,8 +179,10 @@
     domains->clear();
     *params = __res_params{};
     stats->clear();
+    int res_wait_for_pending_req_timeout_count;
     int revision_id = RESOLV_STUB.android_net_res_stats_get_info_for_net(
-            netId, &nscount, res_servers, &dcount, res_domains, params, res_stats);
+            netId, &nscount, res_servers, &dcount, res_domains, params, res_stats,
+            &res_wait_for_pending_req_timeout_count);
 
     // If the netId is unknown (which can happen for valid net IDs for which no DNS servers have
     // yet been configured), there is no revision ID. In this case there is no data to return.
@@ -223,6 +226,8 @@
     for (int i = 0 ; i < dcount ; ++i) {
         domains->push_back(res_domains[i]);
     }
+
+    (*wait_for_pending_req_timeout_count)[0] = res_wait_for_pending_req_timeout_count;
     return 0;
 }
 
@@ -325,12 +330,14 @@
 int ResolverController::getResolverInfo(int32_t netId, std::vector<std::string>* servers,
                                         std::vector<std::string>* domains,
                                         std::vector<std::string>* tlsServers,
-                                        std::vector<int32_t>* params, std::vector<int32_t>* stats) {
+                                        std::vector<int32_t>* params, std::vector<int32_t>* stats,
+                                        std::vector<int32_t>* wait_for_pending_req_timeout_count) {
     using android::net::ResolverStats;
     using android::net::INetd;
     __res_params res_params;
     std::vector<ResolverStats> res_stats;
-    int ret = getDnsInfo(netId, servers, domains, &res_params, &res_stats);
+    int ret = getDnsInfo(netId, servers, domains, &res_params, &res_stats,
+                         wait_for_pending_req_timeout_count);
     if (ret != 0) {
         return ret;
     }
@@ -391,8 +398,10 @@
     std::vector<std::string> domains;
     __res_params params = {};
     std::vector<ResolverStats> stats;
+    std::vector<int32_t> wait_for_pending_req_timeout_count(1, 0);
     time_t now = time(nullptr);
-    int rv = getDnsInfo(netId, &servers, &domains, &params, &stats);
+    int rv = getDnsInfo(netId, &servers, &domains, &params, &stats,
+                        &wait_for_pending_req_timeout_count);
     dw.incIndent();
     if (rv != 0) {
         dw.println("getDnsInfo() failed for netid %u", netId);
@@ -455,6 +464,7 @@
             }
             dw.decIndent();
         }
+        dw.println("Concurrent DNS query timeout: %d", wait_for_pending_req_timeout_count[0]);
     }
     dw.decIndent();
 }