Merge lp:~marcustomlinson/unity-scopes-api/debug_timeouts into lp:unity-scopes-api

[Marcus Tomlinson]

Allow timeout value of -1 to disable the scope idle timeout, reaper timeouts, and two-way invocation timeout.

Increased upper bound of process timeout to allow for slow scope startup when attaching the debugger.

[Paweł Stołowski (stolowski)]

Looks good to me!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:449
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-ci/420/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-utopic-amd64-ci/47
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-utopic-armhf-ci/47/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-utopic-i386-ci/47

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-ci/420/rebuild

review: Needs Fixing (continuous-integration)

Unmerged revisions

=== modified file 'CONFIGFILES'
--- CONFIGFILES	2014-07-25 01:23:02 +0000
+++ CONFIGFILES	2014-08-08 14:08:21 +0000
@@ -84,6 +84,8 @@
   or terminate correctly. If a scope does not respond
   within the allotted time, it is killed with SIGKILL.
 
+  Only values in the range 10 to 15000 milliseconds are accepted.
+
   The default value is 4000 milliseconds.
 
   Note that this value must be less than Locate.Timeout in the [<Middleware>]
@@ -168,6 +170,8 @@
   this range would either cause locate() to falsely conclude that a scope
   could not be started, or cause an unacceptably long wait time.)
 
+  Only values in the range 10 to 15000 milliseconds are accepted.
+
   The default value is 5000 milliseconds.
 
 - Registry.Timeout

=== modified file 'include/unity/scopes/internal/MiddlewareBase.h'
--- include/unity/scopes/internal/MiddlewareBase.h	2014-05-19 08:14:10 +0000
+++ include/unity/scopes/internal/MiddlewareBase.h	2014-08-08 14:08:21 +0000
@@ -82,7 +82,7 @@
     virtual void add_dflt_query_object(QueryObjectBase::SPtr const& query) = 0;
     virtual MWRegistryProxy add_registry_object(std::string const& identity, RegistryObjectBase::SPtr const& registry) = 0;
     virtual MWReplyProxy add_reply_object(ReplyObjectBase::SPtr const& reply) = 0;
-    virtual MWScopeProxy add_scope_object(std::string const& identity, ScopeObjectBase::SPtr const& scope, int64_t idle_timeout = 0) = 0;
+    virtual MWScopeProxy add_scope_object(std::string const& identity, ScopeObjectBase::SPtr const& scope, int64_t idle_timeout = -1) = 0;
     virtual void add_dflt_scope_object(ScopeObjectBase::SPtr const& scope) = 0;
     virtual MWStateReceiverProxy add_state_receiver_object(std::string const& identity, StateReceiverObject::SPtr const& state_receiver) = 0;
 

=== modified file 'include/unity/scopes/internal/zmq_middleware/ObjectAdapter.h'
--- include/unity/scopes/internal/zmq_middleware/ObjectAdapter.h	2014-05-20 04:20:46 +0000
+++ include/unity/scopes/internal/zmq_middleware/ObjectAdapter.h	2014-08-08 14:08:21 +0000
@@ -59,7 +59,7 @@
                   std::string const& endpoint,
                   RequestMode m,
                   int pool_size,
-                  int64_t idle_timeout = 0);
+                  int64_t idle_timeout = -1);
     ~ObjectAdapter();
 
     ZmqMiddleware* mw() const;

=== modified file 'include/unity/scopes/internal/zmq_middleware/ZmqMiddleware.h'
--- include/unity/scopes/internal/zmq_middleware/ZmqMiddleware.h	2014-07-24 10:43:37 +0000
+++ include/unity/scopes/internal/zmq_middleware/ZmqMiddleware.h	2014-08-08 14:08:21 +0000
@@ -102,7 +102,7 @@
                            int64_t timeout);
 
     std::shared_ptr<ObjectAdapter> find_adapter(std::string const& name, std::string const& endpoint_dir,
-                                                std::string const& category, int64_t idle_timeout = 0);
+                                                std::string const& category, int64_t idle_timeout = -1);
 
     ZmqProxy safe_add(std::function<void()>& disconnect_func,
                       std::shared_ptr<ObjectAdapter> const& adapter,

=== modified file 'src/scopes/internal/Reaper.cpp'
--- src/scopes/internal/Reaper.cpp	2014-02-13 02:14:08 +0000
+++ src/scopes/internal/Reaper.cpp	2014-08-08 14:08:21 +0000
@@ -114,17 +114,20 @@
     policy_(p),
     finish_(false)
 {
-    if (reap_interval < 1)
-    {
-        ostringstream s;
-        s << "Reaper: invalid reap_interval (" << reap_interval << "). Interval must be > 0.";
-        throw unity::InvalidArgumentException(s.str());
-    }
-    if (reap_interval > expiry_interval)
-    {
-        ostringstream s;
-        s << "Reaper: reap_interval (" << reap_interval << ") must be <= expiry_interval (" << expiry_interval << ").";
-        throw unity::LogicException(s.str());
+    if (reap_interval != -1 && expiry_interval != -1)
+    {
+        if (reap_interval < 1)
+        {
+            ostringstream s;
+            s << "Reaper: invalid reap_interval (" << reap_interval << "). Interval must be > 0.";
+            throw unity::InvalidArgumentException(s.str());
+        }
+        if (reap_interval > expiry_interval)
+        {
+            ostringstream s;
+            s << "Reaper: reap_interval (" << reap_interval << ") must be <= expiry_interval (" << expiry_interval << ").";
+            throw unity::LogicException(s.str());
+        }
     }
 }
 
@@ -142,7 +145,10 @@
 {
     SPtr reaper(new Reaper(reap_interval, expiry_interval, p));
     reaper->set_self();
-    reaper->start();
+    if (reap_interval != -1 && expiry_interval != -1)
+    {
+        reaper->start();
+    }
     return reaper;
 }
 
@@ -168,7 +174,10 @@
         finish_ = true;
         do_work_.notify_one();
     }
-    reap_thread_.join();
+    if (reap_thread_.joinable())
+    {
+        reap_thread_.join();
+    }
 }
 
 // Add a new entry to the reaper. If the entry is not refreshed within the expiry interval,

=== modified file 'src/scopes/internal/RegistryConfig.cpp'
--- src/scopes/internal/RegistryConfig.cpp	2014-08-04 06:31:27 +0000
+++ src/scopes/internal/RegistryConfig.cpp	2014-08-08 14:08:21 +0000
@@ -75,9 +75,9 @@
         throw ConfigException(configfile + ": " + scoperunner_path_key + " must be an absolute path");
     }
     process_timeout_ = get_optional_int(registry_config_group, process_timeout_key, DFLT_PROCESS_TIMEOUT);
-    if (process_timeout_ < 10 || process_timeout_ > 5000)
+    if (process_timeout_ < 10 || process_timeout_ > 15000)
     {
-        throw_ex("Illegal value (" + to_string(process_timeout_) + ") for " + process_timeout_key + ": value must be 10-5000 ms");
+        throw_ex("Illegal value (" + to_string(process_timeout_) + ") for " + process_timeout_key + ": value must be 10-15000 ms");
     }
 
     KnownEntries const known_entries = {

=== modified file 'src/scopes/internal/RuntimeConfig.cpp'
--- src/scopes/internal/RuntimeConfig.cpp	2014-08-04 06:31:27 +0000
+++ src/scopes/internal/RuntimeConfig.cpp	2014-08-08 14:08:21 +0000
@@ -83,12 +83,12 @@
                                                              default_middleware_ + default_middleware_configfile_key,
                                                              DFLT_MIDDLEWARE_INI);
         reap_expiry_ = get_optional_int(runtime_config_group, reap_expiry_key, DFLT_REAP_EXPIRY);
-        if (reap_expiry_ < 1)
+        if (reap_expiry_ < 1 && reap_expiry_ != -1)
         {
             throw_ex("Illegal value (" + to_string(reap_expiry_) + ") for " + reap_expiry_key + ": value must be > 0");
         }
         reap_interval_ = get_optional_int(runtime_config_group, reap_interval_key, DFLT_REAP_INTERVAL);
-        if (reap_interval_ < 1)
+        if (reap_interval_ < 1 && reap_interval_ != -1)
         {
             throw_ex("Illegal value (" + to_string(reap_interval_) + ") for " + reap_interval_key + ": value must be > 0");
         }

=== modified file 'src/scopes/internal/RuntimeImpl.cpp'
--- src/scopes/internal/RuntimeImpl.cpp	2014-08-01 03:01:40 +0000
+++ src/scopes/internal/RuntimeImpl.cpp	2014-08-08 14:08:21 +0000
@@ -358,13 +358,16 @@
 
     // Create a servant for the scope and register the servant.
     auto scope = unique_ptr<internal::ScopeObject>(new internal::ScopeObject(this, scope_base));
-    int idle_timeout_ms = 0;
     if (!scope_ini_file.empty())
     {
         ScopeConfig scope_config(scope_ini_file);
-        idle_timeout_ms = scope_config.idle_timeout() * 1000;
-    }
-    mw->add_scope_object(scope_id_, move(scope), idle_timeout_ms);
+        int idle_timeout_ms = scope_config.idle_timeout() * 1000;
+        mw->add_scope_object(scope_id_, move(scope), idle_timeout_ms);
+    }
+    else
+    {
+        mw->add_scope_object(scope_id_, move(scope));
+    }
 
     // Inform the registry that this scope is now ready to process requests
     reg_state_receiver->push_state(scope_id_, StateReceiverObject::State::ScopeReady);

=== modified file 'src/scopes/internal/ScopeConfig.cpp'
--- src/scopes/internal/ScopeConfig.cpp	2014-07-25 01:23:02 +0000
+++ src/scopes/internal/ScopeConfig.cpp	2014-08-08 14:08:21 +0000
@@ -152,10 +152,10 @@
 
     // Negative values and values greater than max int (once multiplied by 1000 (s to ms)) are illegal
     const int max_idle_timeout = std::numeric_limits<int>::max() / 1000;
-    if (idle_timeout_ < 0 || idle_timeout_ > max_idle_timeout)
+    if ((idle_timeout_ < 0 || idle_timeout_ > max_idle_timeout) && idle_timeout_ != -1)
     {
         throw_ex("Illegal value (" + std::to_string(idle_timeout_) + ") for " + idle_timeout_key +
-                 ": value must be > 0 and <= " + std::to_string(max_idle_timeout));
+                 ": value must be >= 0 and <= " + std::to_string(max_idle_timeout));
     }
 
 

=== modified file 'src/scopes/internal/zmq_middleware/ObjectAdapter.cpp'
--- src/scopes/internal/zmq_middleware/ObjectAdapter.cpp	2014-07-25 05:04:47 +0000
+++ src/scopes/internal/zmq_middleware/ObjectAdapter.cpp	2014-08-08 14:08:21 +0000
@@ -57,7 +57,7 @@
     endpoint_(endpoint),
     mode_(m),
     pool_size_(pool_size),
-    idle_timeout_(idle_timeout > 0 ? idle_timeout : zmqpp::poller::wait_forever),
+    idle_timeout_(idle_timeout != -1 ? idle_timeout : zmqpp::poller::wait_forever),
     state_(Inactive)
 {
     assert(!name.empty());

=== modified file 'src/scopes/internal/zmq_middleware/ZmqObject.cpp'
--- src/scopes/internal/zmq_middleware/ZmqObject.cpp	2014-07-28 13:06:45 +0000
+++ src/scopes/internal/zmq_middleware/ZmqObject.cpp	2014-08-08 14:08:21 +0000
@@ -281,7 +281,16 @@
 
     zmqpp::poller p;
     p.add(s);
-    p.poll(timeout);
+
+    if (timeout == -1)
+    {
+        p.poll();
+    }
+    else
+    {
+        p.poll(timeout);
+    }
+
     if (!p.has_input(s))
     {
         // If a request times out, we must trash the corresponding socket, otherwise

=== modified file 'test/gtest/scopes/internal/ScopeConfig/ScopeConfig_test.cpp'
--- test/gtest/scopes/internal/ScopeConfig/ScopeConfig_test.cpp	2014-07-29 10:22:45 +0000
+++ test/gtest/scopes/internal/ScopeConfig/ScopeConfig_test.cpp	2014-08-08 14:08:21 +0000
@@ -111,8 +111,8 @@
     }
     catch(ConfigException const& e)
     {
-        boost::regex r("unity::scopes::ConfigException: \".*\": Illegal value \\(-1\\) for IdleTimeout: "
-                       "value must be > 0 and <= [0-9]+");
+        boost::regex r("unity::scopes::ConfigException: \".*\": Illegal value \\(-2\\) for IdleTimeout: "
+                       "value must be >= 0 and <= [0-9]+");
         EXPECT_TRUE(boost::regex_match(e.what(), r));
     }
 }

=== modified file 'test/gtest/scopes/internal/ScopeConfig/bad_timeout.ini.in'
--- test/gtest/scopes/internal/ScopeConfig/bad_timeout.ini.in	2014-06-26 04:56:21 +0000
+++ test/gtest/scopes/internal/ScopeConfig/bad_timeout.ini.in	2014-08-08 14:08:21 +0000
@@ -2,4 +2,4 @@
 DisplayName = Scope name
 Description = Scope description
 Author = Canonical
-IdleTimeout = -1
+IdleTimeout = -2