Fix testTCPClientDisconnectImmediately failed bug : DNSResolve::Cancel bug. Fix EventLoop::QueueInLoop notified_ bug. see Qihoo360#15 Qihoo360#12

Author: zieckey

.travis.yml

@@ -35,9 +35,7 @@ script:
 after_success:
   - cd $CI_HOME
   - pwd
-  - cd build/bin
-  - ./evpp_unittest
-  - cd ../../build-release/bin
-  - ./evpp_unittest
+  - build/bin/evpp_unittest
+  - build-release/bin/evpp_unittest
 #  - coveralls --exclude dependencies --exclude test --exclude include/rpc/msgpack --exclude include/rcp/msgpack.hpp --gcov /usr/bin/gcov-5
 

evpp/dns_resolver.cc

@@ -65,6 +65,7 @@ void DNSResolver::Cancel() {
     assert(loop_->IsInLoopThread());
     if (timer_) {
         timer_->Cancel();
+        timer_.reset();
     }
     functor_ = Functor(); // Release the callback
 }
@@ -95,7 +96,6 @@ void DNSResolver::OnCanceled() {
     evdns_getaddrinfo_cancel(dns_req_);
     dns_req_ = nullptr;
 #endif
-    timer_.reset();
 }
 
 

evpp/event_loop.cc

@@ -161,6 +161,7 @@ void EventLoop::RunInLoop(const Functor& functor) {
 }
 
 void EventLoop::QueueInLoop(const Functor& cb) {
+    //LOG_INFO << "this=" << this << " tid=" << std::this_thread::get_id() << " QueueInLoop notified_=" << notified_.load() << " pending_functor_count_=" << pending_functor_count_;
     {
 #ifdef H_HAVE_BOOST
         auto f = new Functor(cb);
@@ -172,14 +173,21 @@ void EventLoop::QueueInLoop(const Functor& cb) {
 #endif
     }
     ++pending_functor_count_;
-
+    //LOG_INFO << "this=" << this << " tid=" << std::this_thread::get_id() << " QueueInLoop notified_=" << notified_.load() << ", queue a new Functor. pending_functor_count_=" << pending_functor_count_;
     if (!notified_.load()) {
+        //LOG_INFO << "this=" << this << " tid=" << std::this_thread::get_id() << " QueueInLoop call watcher_->Nofity()";
         watcher_->Notify();
-        notified_.store(true);
+
+        //TODO This will cause a bug : miss the notify event and make the functor will never be called.
+        //TODO performance improvement.
+        //notified_.store(true);
+    } else {
+        //LOG_INFO << "this=" << this << " tid=" << std::this_thread::get_id() << " No need to call watcher_->Nofity()";
     }
 }
 
 void EventLoop::DoPendingFunctors() {
+    //LOG_INFO << "this=" << this << " tid=" << std::this_thread::get_id() << " DoPendingFunctors pending_functor_count_=" << pending_functor_count_;
 
 #ifdef H_HAVE_BOOST
     Functor* f = nullptr;
@@ -195,12 +203,14 @@ void EventLoop::DoPendingFunctors() {
         std::lock_guard<std::mutex> lock(mutex_);
         notified_.store(false);
         pending_functors_->swap(functors);
+        //LOG_INFO << "this=" << this << " tid=" << std::this_thread::get_id() << " DoPendingFunctors pending_functor_count_=" << pending_functor_count_ << "notified_=" << notified_.load();
     }
-
+    //LOG_INFO << "this=" << this << " tid=" << std::this_thread::get_id() << " DoPendingFunctors pending_functor_count_=" << pending_functor_count_ << "notified_=" << notified_.load();
     for (size_t i = 0; i < functors.size(); ++i) {
         functors[i]();
         --pending_functor_count_;
     }
+    //LOG_INFO << "this=" << this << " tid=" << std::this_thread::get_id() << " DoPendingFunctors pending_functor_count_=" << pending_functor_count_ << "notified_=" << notified_.load();
 #endif
 }
 

test/dns_resolver_test.cc

@@ -5,39 +5,40 @@
 #include <evpp/dns_resolver.h>
 #include <atomic>
 
-namespace {
-static bool resolved = false;
-static bool deleted = false;
-static void OnResolved(const std::vector <struct in_addr>& addrs) {
-    resolved = true;
-}
-
-static void DeleteDNSResolver(std::shared_ptr<evpp::DNSResolver> r) {
-    deleted = true;
-    r.reset();
-}
-
-}
-
-
 TEST_UNIT(testDNSResolver) {
-    evpp::Duration delay(double(1.0)); // 1s
-    std::unique_ptr<evpp::EventLoopThread> t(new evpp::EventLoopThread);
-    t->Start(true);
-    std::shared_ptr<evpp::DNSResolver> dns_resolver(new evpp::DNSResolver(t->event_loop(), "www.so.com", evpp::Duration(1.0), &OnResolved));
-    dns_resolver->Start();
-
-    while (!resolved) {
-        usleep(1);
+    for (int i = 0; i < 6; i++) {
+        bool resolved = false;
+        bool deleted = false;
+        auto fn_resolved = [&resolved](const std::vector <struct in_addr>& addrs) {
+            LOG_INFO << "Entering fn_resolved";
+            resolved = true;
+        };
+
+        evpp::Duration delay(double(3.0)); // 3s
+        std::unique_ptr<evpp::EventLoopThread> t(new evpp::EventLoopThread);
+        t->Start(true);
+        std::shared_ptr<evpp::DNSResolver> dns_resolver(new evpp::DNSResolver(t->event_loop(), "www.so.com", evpp::Duration(1.0), fn_resolved));
+        dns_resolver->Start();
+
+        while (!resolved) {
+            usleep(1);
+        }
+
+        auto fn_deleter = [&deleted, dns_resolver]() {
+            LOG_INFO << "Entering fn_deleter";
+            deleted = true;
+        };
+
+        t->event_loop()->QueueInLoop(fn_deleter);
+        dns_resolver.reset();
+        while (!deleted) {
+            usleep(1);
+        }
+
+        t->Stop(true);
+        t.reset();
+        if (evpp::GetActiveEventCount() != 0) {
+            H_TEST_ASSERT(evpp::GetActiveEventCount() == 0);
+        }
     }
-
-    t->event_loop()->QueueInLoop(std::bind(&DeleteDNSResolver, dns_resolver));
-    dns_resolver.reset();
-    while (!deleted) {
-        usleep(1);
-    }
-
-    t->Stop(true);
-    t.reset();
-    H_TEST_ASSERT(evpp::GetActiveEventCount() == 0);
 }

test/http_server_test.cc

@@ -160,8 +160,7 @@ static void TestAll() {
 
 
 TEST_UNIT(testHTTPServer1) {
-    for (int j = 0; j < 1000; j++)
-    for (int i = 0; i < 10; ++i) {
+    for (int i = 0; i < 5; ++i) {
         evpp::http::Server ph(i);
         ph.RegisterDefaultHandler(&DefaultRequestHandler);
         ph.RegisterHandler("/push/boot", &RequestHandler);