[trac931] Fix double reads

It was possible to schedule two reads of message length at once. That
actually read two 4-bytes chunks, one from the beginning of the real
message.

src/bin/auth/main.cc

@@ -153,9 +153,14 @@ main(int argc, char* argv[]) {
         cc_session = new Session(io_service.get_io_service());
         cout << "[b10-auth] Configuration session channel created." << endl;
 
+        // We delay starting listening to new commands/config just before we
+        // go into the main loop to avoid confusion due to mixture of
+        // synchronous and asynchronous operations (this would happen in
+        // initializing TSIG keys below).  Until then all operations on the
+        // CC session will take place synchronously.
         config_session = new ModuleCCSession(specfile, *cc_session,
                                              my_config_handler,
-                                             my_command_handler);
+                                             my_command_handler, false);
         cout << "[b10-auth] Configuration channel established." << endl;
 
         xfrin_session = new Session(io_service.get_io_service());
@@ -194,6 +199,10 @@ main(int argc, char* argv[]) {
         cout << "[b10-auth] Loading TSIG keys" << endl;
         isc::server_common::initKeyring(*config_session);
 
+        // Now start asynchronous read.
+        config_session->start();
+        cout << "[b10-auth] Configuration channel started." << endl;
+
         cout << "[b10-auth] Server started." << endl;
         io_service.run();
 

src/lib/config/ccsession.cc

@@ -192,8 +192,10 @@ ModuleCCSession::ModuleCCSession(
     isc::data::ConstElementPtr(*config_handler)(
         isc::data::ConstElementPtr new_config),
     isc::data::ConstElementPtr(*command_handler)(
-        const std::string& command, isc::data::ConstElementPtr args)
+        const std::string& command, isc::data::ConstElementPtr args),
+    bool start_immediately
     ) :
+    started_(false),
     session_(session)
 {
     module_specification_ = readModuleSpecification(spec_file_name);
@@ -237,6 +239,17 @@ ModuleCCSession::ModuleCCSession(
         }
     }
 
+    if (start_immediately) {
+        start();
+    }
+}
+
+void
+ModuleCCSession::start() {
+    if (started_) {
+        isc_throw(CCSessionError, "Module CC session already started");
+    }
+
     // register callback for asynchronous read
     session_.startRead(boost::bind(&ModuleCCSession::startCheck, this));
 }

src/lib/config/ccsession.h

@@ -171,6 +171,11 @@ public:
      * @param command_handler A callback function pointer to be called when
      * a control command from a remote agent needs to be performed on the
      * local module.
+     * @start_immediately If true (default), start listening to new commands
+     * and configuration changes asynchronously at the end of the constructor;
+     * if false, it will be delayed until the start() method is explicitly
+     * called. (This is a short term workaround for an initialization trouble.
+     * We'll need to develop a cleaner solution, and then remove this knob)
      */
     ModuleCCSession(const std::string& spec_file_name,
                     isc::cc::AbstractSession& session,
@@ -178,9 +183,21 @@ public:
                         isc::data::ConstElementPtr new_config) = NULL,
                     isc::data::ConstElementPtr(*command_handler)(
                         const std::string& command,
-                        isc::data::ConstElementPtr args) = NULL
+                        isc::data::ConstElementPtr args) = NULL,
+                    bool start_immediately = true
                     );
 
+    /// Start asynchronous receiving new commands and configuration changes
+    /// asynchronously.
+    ///
+    /// This method must be called only once, and only when the ModuleCCSession
+    /// was constructed with start_immediately being false.  Otherwise
+    /// CCSessionError will be thrown.
+    ///
+    /// As noted in the constructor, this method should be considered a short
+    /// term workaround and will be removed in future.
+    void start();
+
     /**
      * Optional optimization for checkCommand loop; returns true
      * if there are unhandled queued messages in the cc session.
@@ -288,7 +305,8 @@ public:
 private:
     ModuleSpec readModuleSpecification(const std::string& filename);
     void startCheck();
-    
+
+    bool started_;
     std::string module_name_;
     isc::cc::AbstractSession& session_;
     ModuleSpec module_specification_;

src/lib/python/bind10_config.py.in

@@ -41,7 +41,8 @@ def reload():
             DATA_PATH = os.environ["B10_FROM_SOURCE_LOCALSTATEDIR"]
         else:
             DATA_PATH = os.environ["B10_FROM_SOURCE"]
-        PLUGIN_PATHS = [DATA_PATH + '/src/bin/cfgmgr/plugins']
+        PLUGIN_PATHS = [os.environ["B10_FROM_SOURCE"] +
+                            '/src/bin/cfgmgr/plugins']
     else:
         PREFIX = "@prefix@"
         DATA_PATH = "@localstatedir@/@PACKAGE@".replace("${prefix}", PREFIX)

src/lib/server_common/keyring.cc

@@ -30,7 +30,7 @@ void
 updateKeyring(const std::string&, ConstElementPtr data) {
     ConstElementPtr list(data->get("keys"));
     KeyringPtr load(new TSIGKeyRing);
-    for (size_t i(0); i < list->size(); ++ i) {
+    for (size_t i(0); list && i < list->size(); ++ i) {
         load->add(TSIGKey(list->get(i)->stringValue()));
     }
     keyring.swap(load);