Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 92bed612 authored by Ken Chen's avatar Ken Chen
Browse files

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
parent 09b71026

dns_responder/dns_responder.cpp

+15 −0
Original line number Diff line number Diff line
@@ -723,6 +723,13 @@ void DNSResponder::requestHandler() { 
        size_t response_len = sizeof(response);

        if (handleDNSRequest(buffer, len, response, &response_len) &&

            response_len > 0) {

            // place wait_for after handleDNSRequest() so we can check the number of queries in

            // test case before it got responded.

            std::unique_lock guard(cv_mutex_for_deferred_resp_);

            cv_for_deferred_resp_.wait(guard, [this]() REQUIRES(cv_mutex_for_deferred_resp_) {

                return !deferred_resp_;

            });



            len = sendto(socket_, response, response_len, 0,

                         reinterpret_cast<const sockaddr*>(&sa), sa_len);

            std::string host_str =
@@ -916,4 +923,12 @@ bool DNSResponder::makeErrorResponse(DNSHeader* header, ns_rcode rcode, 
    return true;

}



void DNSResponder::setDeferredResp(bool deferred_resp) {

    std::lock_guard<std::mutex> guard(cv_mutex_for_deferred_resp_);

    deferred_resp_ = deferred_resp;

    if (!deferred_resp_) {

        cv_for_deferred_resp_.notify_one();

    }

}



}  // namespace test

dns_responder/dns_responder.h

+5 −0
Original line number Diff line number Diff line
@@ -79,6 +79,7 @@ class DNSResponder { 
    void clearQueries();

    std::condition_variable& getCv() { return cv; }

    std::mutex& getCvMutex() { return cv_mutex_; }

    void setDeferredResp(bool deferred_resp);



  private:

    // Key used for accessing mappings.
@@ -159,6 +160,10 @@ class DNSResponder { 
    std::mutex update_mutex_;

    std::condition_variable cv;

    std::mutex cv_mutex_;



    std::condition_variable cv_for_deferred_resp_;

    std::mutex cv_mutex_for_deferred_resp_;

    bool deferred_resp_ GUARDED_BY(cv_mutex_for_deferred_resp_) = false;

};



}  // namespace test

include/netd_resolv/resolv_stub.h

+2 −1
Original line number Diff line number Diff line
@@ -50,7 +50,8 @@ extern struct ResolvStub { 
    int (*android_net_res_stats_get_info_for_net)(unsigned netid, int* nscount,

                                                  sockaddr_storage servers[MAXNS], int* dcount,

                                                  char domains[MAXDNSRCH][MAXDNSRCHPATH],

                                                  __res_params* params, res_stats stats[MAXNS]);

                                                  __res_params* params, res_stats stats[MAXNS],

                                                  int* wait_for_pending_req_timeout_count);



    void (*android_net_res_stats_get_usable_servers)(const __res_params* params, res_stats stats[],

                                                     int nscount, bool valid_servers[]);

include/netd_resolv/stats.h

+2 −1
Original line number Diff line number Diff line
@@ -51,7 +51,8 @@ LIBNETD_RESOLV_PUBLIC void android_net_res_stats_aggregate(res_stats* stats, int 


LIBNETD_RESOLV_PUBLIC int android_net_res_stats_get_info_for_net(

        unsigned netid, int* nscount, sockaddr_storage servers[MAXNS], int* dcount,

        char domains[MAXDNSRCH][MAXDNSRCHPATH], __res_params* params, res_stats stats[MAXNS]);

        char domains[MAXDNSRCH][MAXDNSRCHPATH], __res_params* params, res_stats stats[MAXNS],

        int* wait_for_pending_req_timeout_count);



// Returns an array of bools indicating which servers are considered good

LIBNETD_RESOLV_PUBLIC void android_net_res_stats_get_usable_servers(const __res_params* params,

res_cache.cpp

+127 −169

File changed.

Preview size limit exceeded, changes collapsed.