[asterisk-scf-commits] asterisk-scf/release/logger.git branch "master" updated.

[Commits to the Asterisk SCF project code repositories]

branch "master" has been updated
       via  3a5e8dcbb6ebd33be6397971509b59557063f8b8 (commit)
      from  7d3ca9ec1f0dd4e2deb1b1b31ea30ed3bf3dc2f0 (commit)

Summary of changes:
 client/src/IceLogger.cpp               |   25 +++++++++++++++++++++----
 include/AsteriskSCF/Logger/IceLogger.h |   22 +++++++++++++++++++---
 2 files changed, 40 insertions(+), 7 deletions(-)


- Log -----------------------------------------------------------------
commit 3a5e8dcbb6ebd33be6397971509b59557063f8b8
Author: David M. Lee <dlee at digium.com>
Date:   Thu Mar 31 16:04:50 2011 -0500

    Fixed race condition in ConfiguredIceLogger ctor
    
    ConfiguredIceLogger was calling updateLoggerFromServiceLocator() in its
    ctor.  This implicitly created a IceUtil::Handle for the object.  If the
    AMI call completed before the ctor finished, then the object would be
    destroyed before the caller could created their own Handle.
    
    The factory functions ensure that a Handle is created prior to invoking
    the AMI call.

diff --git a/client/src/IceLogger.cpp b/client/src/IceLogger.cpp
index c27d05b..4f00e07 100644
--- a/client/src/IceLogger.cpp
+++ b/client/src/IceLogger.cpp
@@ -58,6 +58,23 @@ void IceLogger::logs(const std::string& name, Level logLevel,
     }
 }
 
+IceUtil::Handle<ConfiguredIceLogger> ConfiguredIceLogger::create(
+    const LoggingServerPrx& server) {
+    ConfiguredIceLoggerPtr r = new ConfiguredIceLogger(server);
+    return r;
+}
+
+IceUtil::Handle<ConfiguredIceLogger> ConfiguredIceLogger::create(
+    const Core::Discovery::V1::ServiceLocatorPrx& locator) {
+    // You cannot call updateLoggerFromServiceLocator directly from the ctor.
+    // It implicitly creates a ConfiguredIceLoggerPtr for the AMI call.  If the
+    // AMI call completes before the ctor complete, then the object will be
+    // deleted before the callee can create their own ConfiguredIceLoggerPtr.
+    ConfiguredIceLoggerPtr r = new ConfiguredIceLogger(locator);
+    r->updateLoggerFromServiceLocator();
+    return r;
+}
+
 ConfiguredIceLogger::ConfiguredIceLogger(const LoggingServerPrx& server)
 {
     mLogger.setServer(server);
@@ -66,7 +83,7 @@ ConfiguredIceLogger::ConfiguredIceLogger(const LoggingServerPrx& server)
 ConfiguredIceLogger::ConfiguredIceLogger(const ServiceLocatorPrx& locator) :
     mLocator(locator)
 {
-    updateLoggerFromServiceLocator();
+    // do not call updateLoggerFromServiceLocator.  see comment in create()
 }
 
 void ConfiguredIceLogger::updateLoggerFromServiceLocator()
@@ -88,6 +105,7 @@ void ConfiguredIceLogger::updateLoggerFromServiceLocator()
 
 void ConfiguredIceLogger::locateFinished(const Ice::AsyncResultPtr& r)
 {
+  std::clog << "==> " << this << '\n';
     ServiceLocatorPrx locator = ServiceLocatorPrx::uncheckedCast(r->getProxy());
     try
     {
@@ -171,7 +189,7 @@ ConfiguredIceLoggerPtr AsteriskSCF::System::Logging::createIceLogger(
             "LoggerServer.Proxy");
         if (server)
         {
-            return ConfiguredIceLoggerPtr(new ConfiguredIceLogger(
+            return ConfiguredIceLoggerPtr(ConfiguredIceLogger::create(
                     LoggingServerPrx::checkedCast(server)));
         }
     }
@@ -194,8 +212,7 @@ ConfiguredIceLoggerPtr AsteriskSCF::System::Logging::createIceLogger(
                   << LoggingServerGuid << '\n';
     }
 
-    ConfiguredIceLoggerPtr logger = new ConfiguredIceLogger(locator);
-    logger->updateLoggerFromServiceLocator();
+    ConfiguredIceLoggerPtr logger = ConfiguredIceLogger::create(locator);
 
     IceStorm::TopicManagerPrx topicManager =
         IceStorm::TopicManagerPrx::uncheckedCast(
diff --git a/include/AsteriskSCF/Logger/IceLogger.h b/include/AsteriskSCF/Logger/IceLogger.h
index cc74d06..3719f99 100644
--- a/include/AsteriskSCF/Logger/IceLogger.h
+++ b/include/AsteriskSCF/Logger/IceLogger.h
@@ -48,17 +48,19 @@ class ConfiguredIceLogger : public Discovery::Events
 {
 public:
     /**
-     * Configure an IceLogger directly with the LoggingServer.
+     * Factory function to create an IceLogger directly with the LoggingServer.
      *
      * @param server Server proxy to log to.
      */
-    ConfiguredIceLogger(const LoggingServerPrx& server);
+    static IceUtil::Handle<ConfiguredIceLogger> create(
+        const LoggingServerPrx& server);
     /**
      * Configure an IceLogger which gets LoggingServer from the ServiceLocator.
      *
      * @param locator Locator to use to get the LoggingServer proxy.
      */
-    ConfiguredIceLogger(const Core::Discovery::V1::ServiceLocatorPrx& locator);
+    static IceUtil::Handle<ConfiguredIceLogger> create(
+        const Core::Discovery::V1::ServiceLocatorPrx& locator);
 
     LogOut& getLogger() { return mLogger; }
 
@@ -71,6 +73,20 @@ public:
     void serviceSuspended(const std::string& guid, const Ice::Current&);
     void serviceUnsuspended(const std::string& guid, const Ice::Current&);
 
+protected:
+    /**
+     * Configure an IceLogger directly with the LoggingServer.
+     *
+     * @param server Server proxy to log to.
+     */
+    ConfiguredIceLogger(const LoggingServerPrx& server);
+    /**
+     * Configure an IceLogger which gets LoggingServer from the ServiceLocator.
+     *
+     * @param locator Locator to use to get the LoggingServer proxy.
+     */
+    ConfiguredIceLogger(const Core::Discovery::V1::ServiceLocatorPrx& locator);
+
 private:
     IceLogger mLogger;
     Core::Discovery::V1::ServiceLocatorPrx mLocator;

-----------------------------------------------------------------------


-- 
asterisk-scf/release/logger.git