ChangelogTag: Thu Jul 1 12:16:41 2004 Arvind S. Krishna <arvindk@dre.vanderbilt.edu>

6 files changed, 93 insertions, 119 deletions

diff --git a/TAO/CIAO/ChangeLog b/TAO/CIAO/ChangeLog
index f9a391c26ec..bbd87ddb607 100644
--- a/TAO/CIAO/ChangeLog
+++ b/TAO/CIAO/ChangeLog
@@ -1,3 +1,14 @@
+Thu Jul  1 12:16:41 2004  Arvind S. Krishna  <arvindk@dre.vanderbilt.edu>
+
+        * DnC/ExecutionManager/Execution_Manager.cpp:
+        * DnC/ExecutionManager/ExecutionManager_Impl.h:
+        * DnC/ExecutionManager/ExecutionManager_Impl.cpp:
+        * DnC/NodeManager/Node_Daemon.cpp:
+        * DnC/NodeManager/NodeDaemon_Impl.h:
+        * DnC/NodeManager/NodeDaemon_Impl.cpp:
+
+          Addressed comments left by Ossama after his code review.
+
 Thu Jul  1 12:02:05 2004  Jaiganesh B  <jai@dre.vanderbilt.edu>
 
         * DnC/Config_Handlers/CAD_Handler.cpp:
diff --git a/TAO/CIAO/DnC/ExecutionManager/ExecutionManager_Impl.cpp b/TAO/CIAO/DnC/ExecutionManager/ExecutionManager_Impl.cpp
index ecb2075d71f..65c3ccf99e9 100644
--- a/TAO/CIAO/DnC/ExecutionManager/ExecutionManager_Impl.cpp
+++ b/TAO/CIAO/DnC/ExecutionManager/ExecutionManager_Impl.cpp
@@ -31,81 +31,73 @@ preparePlan (const Deployment::DeploymentPlan &plan,
                    Deployment::StartError
                    ))
 {
-  //@@ Make sure that this call will only called once before destroymanager
-  //   being called!!!!
-
-  // Create a new DomainApplicationMananager servant
-  ACE_NEW_THROW_EX (this->dam_servant_,
-                    CIAO::DomainApplicationManager_Impl
-                    (this->orb_.in (),
-                     this->poa_.in (),
-                     Deployment::TargetManager::_nil (),
-                     plan,
-                     this->init_file_.c_str ()),
-                    CORBA::NO_MEMORY ());
-
-  ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil());
-
-
-  // @@ (OO) If this method is ever called twice in a row, you're
-  //         going to leak the previous instance since you don't
-  //         decrease the reference count.  You may want to consider
-  //         caching this->dam_servant_ into a
-  //         TAO::Utils::Servant_Var<> to ease memory management,
-  //         e.g. automatically decreasing reference counts, etc.
-
-  /**
-   *===================================================================
-   * MAIN STEP: This call parses the deployment plan, generates the Node
-   * specific plan and starts the deployment process
-   *===================================================================
-   */
-  this->dam_servant_->init (ACE_ENV_SINGLE_ARG_PARAMETER);
-  // @@ (OO) You're missing an
-  //           ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil())
-  //         here.  Emulated exceptions won't function correctly
-  //         without it.
-
-  this->dam_servant_->set_uuid (plan.UUID.in ());
-
-  // Register with our POA and activate the object.
-  PortableServer::ObjectId_var oid
-    = this->poa_->activate_object (this->dam_servant_
-                                   ACE_ENV_ARG_PARAMETER);
-
-  ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil ());
-
-  // Get the reference of the object.
-  CORBA::Object_var objref
-    = this->poa_->id_to_reference (oid.in ()
-                                   ACE_ENV_ARG_PARAMETER);
-  ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil ());
-
-  this->dam_ =
-    Deployment::DomainApplicationManager::_narrow (objref.in ()
+  // As DAnCE currently supports only one domain for now, we return the same
+  // DomainApplicationManager for multiple calls to prepare plan
+
+  if (this->dam_servant_ == 0)
+    {
+      // Create a new DomainApplicationMananager servant
+      ACE_NEW_THROW_EX (this->dam_servant_,
+                        CIAO::DomainApplicationManager_Impl
+                        (this->orb_.in (),
+                         this->poa_.in (),
+                         Deployment::TargetManager::_nil (),
+                         plan,
+                         this->init_file_.c_str ()),
+                        CORBA::NO_MEMORY ());
+
+      ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil());
+
+      /**
+       *===================================================================
+       * MAIN STEP: This call parses the deployment plan, generates the Node
+       * specific plan and starts the deployment process
+       *===================================================================
+       */
+      this->dam_servant_->init (ACE_ENV_SINGLE_ARG_PARAMETER);
+      ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil());
+
+      this->dam_servant_->set_uuid (plan.UUID.in ());
+
+      // Register with our POA and activate the object.
+      PortableServer::ObjectId_var oid
+        = this->poa_->activate_object (this->dam_servant_.in ()
+                                       ACE_ENV_ARG_PARAMETER);
+
+      ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil ());
+
+      // Get the reference of the object.
+      CORBA::Object_var objref
+        = this->poa_->id_to_reference (oid.in ()
+                                       ACE_ENV_ARG_PARAMETER);
+      ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil ());
+
+      this->dam_ =
+        Deployment::DomainApplicationManager::_narrow (objref.in ()
                                                    ACE_ENV_ARG_PARAMETER);
-  ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil ());
+      ACE_CHECK_RETURN (Deployment::DomainApplicationManager::_nil ());
+    }
 
   // Return the ApplicationManager instance
   return Deployment::DomainApplicationManager::_duplicate (this->dam_.in ());
 }
 
-
 Deployment::DomainApplicationManagers *
 CIAO::ExecutionManager_Impl::getManagers (ACE_ENV_SINGLE_ARG_DECL)
   ACE_THROW_SPEC ((CORBA::SystemException))
 {
-  // @@ (OO) Since you're not doing anything with the allocated
-  //         sequence, there is no need to store in a "_var".  Simply
-  //         return the pointer.
-
   // Initialize the list of DomainApplication Managers
   Deployment::DomainApplicationManagers_var list;
-  /*ACE_NEW_THROW_EX (list,
+  ACE_NEW_THROW_EX (list,
                     Deployment::DomainApplicationManagers,
                     CORBA::NO_MEMORY());
-  ACE_CHECK_RETURN (0)
-  */
+  ACE_CHECK_RETURN (0);
+
+  // Add the manager to the list
+  CORBA::ULong index = list->length ();
+  list->length (index + 1);
+  list [index] =
+    Deployment::DomainApplicationManager::_duplicate (this->dam_.in ());
   return list._retn ();
 }
 
@@ -132,11 +124,9 @@ CIAO::ExecutionManager_Impl::destroyManager (Deployment::DomainApplicationManage
     this->poa_->deactivate_object (oid.in ()
                                    ACE_ENV_ARG_PARAMETER);
 
-    // @@ (OO) Will this this->dam_servant_ ever be used again after
-    //         the reference count is decreased?  If not, you may want
-    //         to set it to zero to force it to be in a consistent
-    //         state.
-    this->dam_servant_->_remove_ref ();
+    // set it to zero to force it to be in a consistent
+    // state.
+    this->dam_servant_ = 0;
     this->dam_ = Deployment::DomainApplicationManager::_nil ();
 
     ACE_TRY_CHECK;
@@ -160,5 +150,5 @@ CIAO::ExecutionManager_Impl::shutdown (ACE_ENV_SINGLE_ARG_DECL)
   //         Please use ACE_ENV_ARG_PARAMETER instead.
 
   // Shutdown the ORB on which it is runing
-  this->orb_->shutdown (0 ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS);
+  this->orb_->shutdown (0 ACE_ENV_ARG_PARAMETER);
 }
diff --git a/TAO/CIAO/DnC/ExecutionManager/ExecutionManager_Impl.h b/TAO/CIAO/DnC/ExecutionManager/ExecutionManager_Impl.h
index 65f212916ce..619d5d7f3ac 100644
--- a/TAO/CIAO/DnC/ExecutionManager/ExecutionManager_Impl.h
+++ b/TAO/CIAO/DnC/ExecutionManager/ExecutionManager_Impl.h
@@ -23,6 +23,7 @@
 
 #include "ace/Synch.h"
 #include "ace/Hash_Map_Manager.h"
+#include "tao/Utils/Servant_Var.h"
 #include "DomainApplicationManager/DomainApplicationManager_Impl.h"
 
 /**
@@ -120,12 +121,8 @@ namespace CIAO
 
       Deployment::DomainApplicationManager_var dam_;
 
-      // @@ (OO) You may want to consider caching this->dam_servant_
-      //         into a TAO::Utils::Servant_Var<> to ease memory
-      //         management, e.g. automatically decreasing reference
-      //         counts, etc.
       /// Cached servant of the DomainApplicaitonmanager.
-      CIAO::DomainApplicationManager_Impl * dam_servant_;
+      TAO::Utils::Servant_Var<CIAO::DomainApplicationManager_Impl> dam_servant_;
     };
 
 };
diff --git a/TAO/CIAO/DnC/ExecutionManager/Execution_Manager.cpp b/TAO/CIAO/DnC/ExecutionManager/Execution_Manager.cpp
index 9b7c3eb9180..1815533921c 100644
--- a/TAO/CIAO/DnC/ExecutionManager/Execution_Manager.cpp
+++ b/TAO/CIAO/DnC/ExecutionManager/Execution_Manager.cpp
@@ -49,7 +49,6 @@ parse_args (int argc, char *argv[])
 {
   ACE_Get_Opt get_opts (argc, argv, "o:c:m:i:n");
   int c;
-
   while ((c = get_opts ()) != -1)
     switch (c)
       {
diff --git a/TAO/CIAO/DnC/NodeManager/NodeDaemon_Impl.cpp b/TAO/CIAO/DnC/NodeManager/NodeDaemon_Impl.cpp
index 6f3ff604ea8..1012a9bf4b3 100644
--- a/TAO/CIAO/DnC/NodeManager/NodeDaemon_Impl.cpp
+++ b/TAO/CIAO/DnC/NodeManager/NodeDaemon_Impl.cpp
@@ -7,11 +7,10 @@
 //         versions of emulated exception parameters.  Please remove
 //         the "_WITH_DEFAULTS"
 CIAO::NodeDaemon_Impl::NodeDaemon_Impl (const char *name,
-                                    CORBA::ORB_ptr orb,
-                                    PortableServer::POA_ptr poa,
-                                    const char * nodapp_loc,
-                                    int spawn_delay
-                                    ACE_ENV_ARG_DECL_WITH_DEFAULTS)
+                                        CORBA::ORB_ptr orb,
+                                        PortableServer::POA_ptr poa,
+                                        const char * nodapp_loc,
+                                        int spawn_delay)
   ACE_THROW_SPEC ((CORBA::SystemException))
   : orb_ (CORBA::ORB::_duplicate (orb)),
     poa_ (PortableServer::POA::_duplicate (poa)),
@@ -72,43 +71,29 @@ CIAO::NodeDaemon_Impl::shutdown (ACE_ENV_SINGLE_ARG_DECL)
   ACE_CHECK;
 }
 
-// @@ (OO) Method definitions should never use "_WITH_DEFAULTS"
-//         versions of emulated exception parameters.  Please remove
-//         the "_WITH_DEFAULTS"
-// @@ (OO) In this case, you should use the "_NOT_USED" version
-//         instead.
 void
 CIAO::NodeDaemon_Impl::joinDomain (const Deployment::Domain & ,
-                               Deployment::TargetManager_ptr ,
-                               Deployment::Logger_ptr
-                               ACE_ENV_ARG_DECL_WITH_DEFAULTS)
+                                   Deployment::TargetManager_ptr ,
+                                   Deployment::Logger_ptr)
   ACE_THROW_SPEC ((CORBA::SystemException))
 {
-
+  ACE_THROW (CORBA::NO_IMPLEMENT ());
 }
 
-// @@ (OO) Method definitions should never use "_WITH_DEFAULTS"
-//         versions of emulated exception parameters.  Please remove
-//         the "_WITH_DEFAULTS"
-// @@ (OO) In this case, you should use the "_NOT_USED" version
-//         instead.
 void
-CIAO::NodeDaemon_Impl::leaveDomain (ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS)
+CIAO::NodeDaemon_Impl::leaveDomain (ACE_ENV_SINGLE_ARG_DECL)
   ACE_THROW_SPEC ((CORBA::SystemException))
 {
   //Implementation undefined.
+  ACE_THROW (CORBA::NO_IMPLEMENT ());
 }
 
 
-// @@ (OO) Method definitions should never use "_WITH_DEFAULTS"
-//         versions of emulated exception parameters.  Please remove
-//         the "_WITH_DEFAULTS"
 Deployment::NodeApplicationManager_ptr
-CIAO::NodeDaemon_Impl::preparePlan (const Deployment::DeploymentPlan &plan
-                                    ACE_ENV_ARG_DECL_WITH_DEFAULTS)
+CIAO::NodeDaemon_Impl::preparePlan (const Deployment::DeploymentPlan &plan)
   ACE_THROW_SPEC ((CORBA::SystemException,
-                 Deployment::StartError,
-                 Deployment::PlanError))
+                   Deployment::StartError,
+                   Deployment::PlanError))
 {
   // Return cached manager
   ACE_TRY
@@ -125,7 +110,7 @@ CIAO::NodeDaemon_Impl::preparePlan (const Deployment::DeploymentPlan &plan
 
       PortableServer::ServantBase_var safe (app_mgr);
 
-           //@@ Note: after the init call the servant ref count would become 2. so
+     //@@ Note: after the init call the servant ref count would become 2. so
      //   we can leave the safeservant along and be dead. Also note that I added
       this->manager_ =
             app_mgr->init (this->nodeapp_location_,
@@ -163,12 +148,8 @@ CIAO::NodeDaemon_Impl::preparePlan (const Deployment::DeploymentPlan &plan
   ACE_CHECK_RETURN (0);
 }
 
-// @@ (OO) Method definitions should never use "_WITH_DEFAULTS"
-//         versions of emulated exception parameters.  Please remove
-//         the "_WITH_DEFAULTS"
 void
-CIAO::NodeDaemon_Impl::destroyManager (Deployment::NodeApplicationManager_ptr
-                                       ACE_ENV_ARG_DECL_WITH_DEFAULTS)
+CIAO::NodeDaemon_Impl::destroyManager (Deployment::NodeApplicationManager_ptr)
   ACE_THROW_SPEC ((CORBA::SystemException, Deployment::StopError))
 {
   ACE_TRY
diff --git a/TAO/CIAO/DnC/NodeManager/NodeDaemon_Impl.h b/TAO/CIAO/DnC/NodeManager/NodeDaemon_Impl.h
index 2b9dcc00107..7802f84f964 100644
--- a/TAO/CIAO/DnC/NodeManager/NodeDaemon_Impl.h
+++ b/TAO/CIAO/DnC/NodeManager/NodeDaemon_Impl.h
@@ -45,26 +45,14 @@ namespace CIAO
   {
   public:
 
-    // @@ (OO) In general, we only use _WITH_DEFAULTS for IDL defined
-    ///        methods.  Please remove _WITH_DEFAULT from the emulated
-    ///        exception parameter below.
     /// Constructor
     NodeDaemon_Impl (const char *name,
                      CORBA::ORB_ptr orb,
                      PortableServer::POA_ptr p,
                      const char * nodeapp_loc,
-                     int spawn_delay
-                     ACE_ENV_ARG_DECL_WITH_DEFAULTS)
+                     int spawn_delay)
                 ACE_THROW_SPEC ((CORBA::SystemException));
 
-    // @@ (OO) Since this class is reference counted, please make this
-    //         destructor protected to enforce proper memory managment
-    //         through the reference counting mechanism (i.e. to
-    //         disallow calling operator delete() on an instance of
-    //         this class.
-    /// Destructor
-    virtual ~NodeDaemon_Impl (void);
-
     /// Get the containing POA.  This operation does *not*
     /// increase the reference count of the POA.
     virtual PortableServer::POA_ptr _default_POA (void);
@@ -100,6 +88,14 @@ namespace CIAO
                        Deployment::StopError));
 
   protected:
+    // Since this class is reference counted, making this
+    // destructor protected to enforce proper memory managment
+    // through the reference counting mechanism (i.e. to
+    // disallow calling operator delete() on an instance of
+    // this class.
+    /// Destructor
+    virtual ~NodeDaemon_Impl (void);
+
     // Keep a pointer to the managing ORB serving this servant.
     CORBA::ORB_var orb_;