From 65c5013ecca3ecbb3a03631166c35fd8c37f6d94 Mon Sep 17 00:00:00 2001
From: Mathieu Stefani <mathieu@datacratic.com>
Date: Tue, 30 Sep 2014 13:40:02 -0400
Subject: [PATCH] Redis: Added a lock in the event loop before reading the
 earliestTimeout. Also improved the timeout test case

---
 service/redis.cc                    | 12 +++++++++---
 service/testing/redis_async_test.cc | 12 ++++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/service/redis.cc b/service/redis.cc
index c877dc1a..f09f9645 100644
--- a/service/redis.cc
+++ b/service/redis.cc
@@ -500,10 +500,16 @@ struct AsyncConnection::EventLoop {
             //sleep(1);
             Date now = Date::now();
 
-            if (connection->earliestTimeout < now)
+            Date earliestTimeout;
+            {
+                std::lock_guard<Lock> guard(connection->lock);
+                earliestTimeout = connection->earliestTimeout;
+            }
+
+            if (earliestTimeout < now)
                 connection->expireTimeouts(now);
 
-            double timeLeft = now.secondsUntil(connection->earliestTimeout);
+            double timeLeft = now.secondsUntil(earliestTimeout);
 
             //cerr << "timeLeft = " << timeLeft << endl;
             //cerr << "fds[0].events = " << fds[0].events << endl;
@@ -512,7 +518,7 @@ struct AsyncConnection::EventLoop {
             int timeout = std::min(1000.0,
                                    std::max<double>(0, 1000 * timeLeft));
 
-            if (connection->earliestTimeout == Date::positiveInfinity())
+            if (earliestTimeout == Date::positiveInfinity())
                 timeout = 1000000;
 
             //cerr << "looping; fd0 = " << fds[1].fd << " timeout = "
diff --git a/service/testing/redis_async_test.cc b/service/testing/redis_async_test.cc
index 0fdd70dd..6453d0f8 100644
--- a/service/testing/redis_async_test.cc
+++ b/service/testing/redis_async_test.cc
@@ -322,23 +322,34 @@ BOOST_AUTO_TEST_CASE( test_redis_timeout )
         auto result = connection.exec(GET("Hello"), 2.0);
         BOOST_CHECK(result.timedOut());
 
+        bool gotResult = false;
+
+        Date before = Date::now();
+
         auto onResult = [&](const Redis::Result &result) {
             BOOST_CHECK(result.timedOut());
+            const auto timeElapsed = Date::now().secondsSince(before);
+            BOOST_CHECK(timeElapsed >= 2.0);
+            gotResult = true;
         };
 
         connection.queue(GET("Hello"), onResult, 2.0);
         ML::sleep(5.0);
+        BOOST_CHECK(gotResult);
     }
 
     std::cerr << "Resuming redis" << std::endl;
     redis.resume();
 
     {
+        bool gotResult = false;
+
         auto onResult = [&](const Redis::Result &result) {
             if (result) {
                 auto reply = result.reply();
                 BOOST_CHECK_EQUAL(reply.type(), Redis::STRING);
                 BOOST_CHECK_EQUAL(reply.asString(), "World");
+                gotResult = true;
             }
             else {
                 BOOST_CHECK(false);
@@ -347,6 +358,7 @@ BOOST_AUTO_TEST_CASE( test_redis_timeout )
 
         connection.queue(GET("Hello"), onResult);
         ML::sleep(2.0);
+        BOOST_CHECK(gotResult);
     }
 
     redis.shutdown();