diff --git a/CEP/BB/BBSControl/include/BBSControl/BBSMultiStep.h b/CEP/BB/BBSControl/include/BBSControl/BBSMultiStep.h index 4119a54d0ae0573db87a79639b9a20055abafd21..cd1a6950236f4262b5d3d3976a2b32cc2ead318f 100644 --- a/CEP/BB/BBSControl/include/BBSControl/BBSMultiStep.h +++ b/CEP/BB/BBSControl/include/BBSControl/BBSMultiStep.h @@ -88,7 +88,8 @@ namespace LOFAR // Implementation of getAllSteps() for BBSMultiStep. It retrieves all // steps by calling getAllSteps() on all steps that comprise this // multistep. - virtual void doGetAllSteps(vector<const BBSStep*>& steps) const; + virtual void + doGetAllSteps(vector< shared_ptr<const BBSStep> >& steps) const; // Check to see if there's an infinite recursion present in the // definition of a BBSMultiStep. This can happen when one of the steps @@ -97,7 +98,7 @@ namespace LOFAR void infiniteRecursionCheck(const string& name) const; // Vector holding a sequence of BBSSteps. - vector<const BBSStep*> itsSteps; + vector< shared_ptr<const BBSStep> > itsSteps; }; // @} diff --git a/CEP/BB/BBSControl/include/BBSControl/BBSStep.h b/CEP/BB/BBSControl/include/BBSControl/BBSStep.h index a3009de2ac6f83e8f4df6afea3d89dfdefcdfc23..7de271d7b24337c2592c314c9436894f94b8f726 100644 --- a/CEP/BB/BBSControl/include/BBSControl/BBSStep.h +++ b/CEP/BB/BBSControl/include/BBSControl/BBSStep.h @@ -33,6 +33,7 @@ #include <Common/lofar_vector.h> #include <Common/lofar_iosfwd.h> #include <Common/LofarTypes.h> +#include <Common/lofar_smartptr.h> namespace LOFAR { @@ -61,7 +62,8 @@ namespace LOFAR // cause the pointer to never be freed. This can happen since a BBSStep // stores a pointer to its parent as backreference. Here, we should // probably use a boost::weak_ptr. See Bug #906 - class BBSStep : public Command + class BBSStep : public Command, + public enable_shared_from_this<BBSStep> { public: // Destructor. @@ -80,7 +82,7 @@ namespace LOFAR // class that can be used to iterate over the all steps. I had some // trouble getting that thingy working, so, due to time constraints, I // implemented things the ugly way. - vector<const BBSStep*> getAllSteps() const; + vector< shared_ptr<const BBSStep> > getAllSteps() const; // Create a new step object. The new step can either be a BBSSingleStep // or a BBSMultiStep object. This is determined by examining the @@ -88,9 +90,9 @@ namespace LOFAR // <tt>Step.<em>name</em>.Steps</tt>, then \a aName is a BBSMultiStep, // otherwise it is a SingleStep. The third, optional, argument is used // to pass a backreference to the parent BBSStep object. - static BBSStep* create(const string& name, - const ACC::APS::ParameterSet& parSet, - const BBSStep* parent = 0); + static shared_ptr<BBSStep> create(const string& name, + const ACC::APS::ParameterSet& parSet, + const BBSStep* parent = 0); // Print the contents of \c *this in human readable form into the output // stream \a os. @@ -153,7 +155,8 @@ namespace LOFAR // Implementation of getAllSteps(). The default implementation adds \c // this to the vector \a steps. // \note This method must be overridden by BBSMultiStep. - virtual void doGetAllSteps(vector<const BBSStep*>& steps) const; + virtual void + doGetAllSteps(vector< shared_ptr<const BBSStep> >& steps) const; // Name of this step. string itsName; @@ -192,16 +195,7 @@ namespace LOFAR }; - // Factory that can be used to generate new BBSStep objects. - // The factory is defined as a singleton. - typedef Singleton < - ObjectFactory < BBSStep(const string&, - const ACC::APS::ParameterSet&, - const BBSStep*), - string > - > BBSStepFactory; - - + } // namespace BBS } // namespace LOFAR diff --git a/CEP/BB/BBSControl/include/BBSControl/BBSStrategy.h b/CEP/BB/BBSControl/include/BBSControl/BBSStrategy.h index f0b9fb9465856b74dab6e275fa682702f3c16baf..5a8255c76703abb44c44b56c3f6304a699f11bb6 100644 --- a/CEP/BB/BBSControl/include/BBSControl/BBSStrategy.h +++ b/CEP/BB/BBSControl/include/BBSControl/BBSStrategy.h @@ -32,6 +32,7 @@ #include <Common/lofar_iosfwd.h> #include <Common/lofar_string.h> #include <Common/lofar_vector.h> +#include <Common/lofar_smartptr.h> namespace LOFAR { @@ -79,7 +80,7 @@ namespace LOFAR // done in pre-order, depth-first. // \todo Do we really want to implement such "iterator-like behaviour" // in this class? - vector<const BBSStep*> getAllSteps() const; + vector< shared_ptr<const BBSStep> > getAllSteps() const; // Indicate whether the BBSSteps contained in \c itsSteps should also be // written when write(ParameterSet&) is called. @@ -140,7 +141,7 @@ namespace LOFAR Integration itsIntegration; // Sequence of steps that comprise this solve strategy. - vector<const BBSStep*> itsSteps; + vector< shared_ptr<const BBSStep> > itsSteps; // Flag indicating whether the BBSStep objects in \c itsSteps should // also be written when write(ParameterSet&) is called. diff --git a/CEP/BB/BBSControl/include/BBSControl/CommandQueue.h b/CEP/BB/BBSControl/include/BBSControl/CommandQueue.h index 52eb496ca01684c5e2ab5347739e71bd4b867599..cd8c4eb4dfa9c8d40753f081ba5a144d93bd5759 100644 --- a/CEP/BB/BBSControl/include/BBSControl/CommandQueue.h +++ b/CEP/BB/BBSControl/include/BBSControl/CommandQueue.h @@ -189,7 +189,6 @@ namespace LOFAR // // \todo Wrap multiple queries (needed for, e.g., reconstructing a // BBSSolveStep) in one transaction. -// const Command* getNextCommand(); pair<shared_ptr<const Command>, const CommandId> getNextCommand() const; // Set the BBSStrategy in the command queue. All information, \e except diff --git a/CEP/BB/BBSControl/include/BBSControl/GlobalProcessControl.h b/CEP/BB/BBSControl/include/BBSControl/GlobalProcessControl.h index d6e444d184420ad43868966399fd531c806436ba..d856bd1bcba3d9ae764e4d603e756133c261d046 100644 --- a/CEP/BB/BBSControl/include/BBSControl/GlobalProcessControl.h +++ b/CEP/BB/BBSControl/include/BBSControl/GlobalProcessControl.h @@ -28,6 +28,7 @@ //# Includes #include <PLC/ProcessControl.h> +#include <Common/lofar_smartptr.h> #include <BBSControl/CommandId.h> #include <BBSControl/CommandResult.h> @@ -120,13 +121,13 @@ namespace LOFAR // Vector containing all the separate steps, in sequential order, that // the strategy consists of. - vector<const BBSStep*> itsSteps; + vector< shared_ptr<const BBSStep> > itsSteps; // Iterator for keeping track where we left while traversing the vector // \c itsSteps. We need this iterator, because the run() method will be // invoked several times by ACCmain. In each call to run() we must // execute one BBSStep. - vector<const BBSStep*>::const_iterator itsStepsIterator; + vector< shared_ptr<const BBSStep> >::const_iterator itsStepsIterator; // CommandQueue where strategies and steps can be "posted". scoped_ptr<CommandQueue> itsCommandQueue; diff --git a/CEP/BB/BBSControl/src/BBSMultiStep.cc b/CEP/BB/BBSControl/src/BBSMultiStep.cc index 659989e2ae7a2d7d501df8a5bcb92d58b9d3562a..dee46317793de36fb6ed82bb91924000f9e2a7de 100644 --- a/CEP/BB/BBSControl/src/BBSMultiStep.cc +++ b/CEP/BB/BBSControl/src/BBSMultiStep.cc @@ -59,12 +59,6 @@ namespace LOFAR BBSMultiStep::~BBSMultiStep() { LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, ""); - - // Clean up all steps. - for(uint i = 0; i < itsSteps.size(); ++i) { - delete itsSteps[i]; - } - itsSteps.clear(); } @@ -139,11 +133,12 @@ namespace LOFAR } - void BBSMultiStep::doGetAllSteps(vector<const BBSStep*>& steps) const + void + BBSMultiStep::doGetAllSteps(vector< shared_ptr<const BBSStep> >& steps) const { LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, ""); for (uint i = 0; i < itsSteps.size(); ++i) { - vector<const BBSStep*> substeps = itsSteps[i]->getAllSteps(); + vector< shared_ptr<const BBSStep> > substeps = itsSteps[i]->getAllSteps(); steps.insert(steps.end(), substeps.begin(), substeps.end()); } } diff --git a/CEP/BB/BBSControl/src/BBSStep.cc b/CEP/BB/BBSControl/src/BBSStep.cc index 525876247556092e25d0e7bab9d33d1eb4c4e671..d36f1be8e9cd76e1d353aad6bfaa23c9243fbb2c 100644 --- a/CEP/BB/BBSControl/src/BBSStep.cc +++ b/CEP/BB/BBSControl/src/BBSStep.cc @@ -63,26 +63,27 @@ namespace LOFAR } - vector<const BBSStep*> BBSStep::getAllSteps() const + vector< shared_ptr<const BBSStep> > BBSStep::getAllSteps() const { LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, ""); - vector<const BBSStep*> steps; + vector< shared_ptr<const BBSStep> > steps; doGetAllSteps(steps); return steps; } - BBSStep* BBSStep::create(const string& name, - const ParameterSet& parset, - const BBSStep* parent) + shared_ptr<BBSStep> BBSStep::create(const string& name, + const ParameterSet& parset, + const BBSStep* parent) { LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, ""); + shared_ptr<BBSStep> step; // If \a parset contains a key <tt>Step.<em>name</em>.Steps</tt>, then // \a name is a BBSMultiStep, otherwise it is a SingleStep. if (parset.isDefined("Step." + name + ".Steps")) { LOG_TRACE_COND_STR(name << " is a MultiStep"); - return new BBSMultiStep(name, parset, parent); + step.reset(new BBSMultiStep(name, parset, parent)); } else { LOG_TRACE_COND_STR(name << " is a SingleStep"); // We'll have to figure out what kind of SingleStep we must @@ -92,23 +93,24 @@ namespace LOFAR toUpper(parset.getString("Step." + name + ".Operation")); LOG_TRACE_COND_STR("Creating a " << oper << " step ..."); if (oper == "SOLVE") - return new BBSSolveStep(name, parset, parent); + step.reset(new BBSSolveStep(name, parset, parent)); else if (oper == "SUBTRACT") - return new BBSSubtractStep(name, parset, parent); + step.reset(new BBSSubtractStep(name, parset, parent)); else if (oper == "CORRECT") - return new BBSCorrectStep(name, parset, parent); + step.reset(new BBSCorrectStep(name, parset, parent)); else if (oper == "PREDICT") - return new BBSPredictStep(name, parset, parent); + step.reset(new BBSPredictStep(name, parset, parent)); else if (oper == "SHIFT") - return new BBSShiftStep(name, parset, parent); + step.reset(new BBSShiftStep(name, parset, parent)); else if (oper == "REFIT") - return new BBSRefitStep(name, parset, parent); + step.reset(new BBSRefitStep(name, parset, parent)); else THROW (BBSControlException, "Operation \"" << oper << "\" is not a valid Step operation"); } catch (APSException& e) { THROW (BBSControlException, e.what()); } } + return step; } @@ -213,10 +215,11 @@ namespace LOFAR //##-------- P r i v a t e m e t h o d s --------##// - void BBSStep::doGetAllSteps(vector<const BBSStep*>& steps) const + void + BBSStep::doGetAllSteps(vector< shared_ptr<const BBSStep> >& steps) const { LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, ""); - steps.push_back(this); + steps.push_back(shared_from_this()); } diff --git a/CEP/BB/BBSControl/src/BBSStrategy.cc b/CEP/BB/BBSControl/src/BBSStrategy.cc index 491228a1d6ea8e0561a7903a3ec3b8f088443855..13c30ad4848ab10d87ee4b8e34b4c3f6eb6536d4 100644 --- a/CEP/BB/BBSControl/src/BBSStrategy.cc +++ b/CEP/BB/BBSControl/src/BBSStrategy.cc @@ -113,12 +113,6 @@ namespace LOFAR BBSStrategy::~BBSStrategy() { LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, ""); - - // Clean up all steps. - for (uint i = 0; i < itsSteps.size(); ++i) { - delete itsSteps[i]; - } - itsSteps.clear(); } @@ -147,12 +141,13 @@ namespace LOFAR } - vector<const BBSStep*> BBSStrategy::getAllSteps() const + vector< shared_ptr<const BBSStep> > BBSStrategy::getAllSteps() const { LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, ""); - vector<const BBSStep*> steps; + vector< shared_ptr<const BBSStep> > steps; for (uint i = 0; i < itsSteps.size(); ++i) { - vector<const BBSStep*> substeps = itsSteps[i]->getAllSteps(); + vector< shared_ptr<const BBSStep> > substeps = + itsSteps[i]->getAllSteps(); steps.insert(steps.end(), substeps.begin(), substeps.end()); } return steps; diff --git a/CEP/BB/BBSControl/src/CommandQueue.cc b/CEP/BB/BBSControl/src/CommandQueue.cc index 3137f3dac8dc41e06f2832431bb71b066c3d874f..a2bf3656f7d5bbbbcaa1698d19ed32831bdbb21e 100644 --- a/CEP/BB/BBSControl/src/CommandQueue.cc +++ b/CEP/BB/BBSControl/src/CommandQueue.cc @@ -24,23 +24,18 @@ #include <lofar_config.h> //# Includes -#include <BBSControl/CommandQueue.h> +#include <BBSControl/Command.h> #include <BBSControl/CommandId.h> +#include <BBSControl/CommandQueue.h> #include <BBSControl/CommandResult.h> #include <BBSControl/LocalControlId.h> -// #include <BBSControl/CommandQueueTransactors.h> -// #include <BBSControl/CommandQueueTrigger.h> #include <BBSControl/QueryBuilder/AddCommand.h> -#include <BBSControl/BBSSingleStep.h> -#include <BBSControl/BBSSolveStep.h> +#include <BBSControl/BBSStep.h> #include <BBSControl/BBSStrategy.h> -#include <BBSControl/BBSStructs.h> #include <BBSControl/Exceptions.h> #include <APS/ParameterSet.h> #include <APS/Exceptions.h> -#include <Common/StreamUtil.h> #include <Common/LofarLogger.h> -#include <Common/lofar_typeinfo.h> //# Now here's an ugly kludge: libpqxx defines four different top-level //# exception classes. In order to avoid a lot of code duplication we clumped @@ -136,9 +131,6 @@ namespace LOFAR { LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, ""); - Command* command(0); - CommandId id; - // Get the next command from the command queue. This call will only // return the command type. ParameterSet ps = @@ -147,10 +139,10 @@ namespace LOFAR // If command type is an empty string, then we've probably received an // empty result row; return a null pointer. string type = ps.getString("Type"); - if (type.empty()) return make_pair(command, id); + if (type.empty()) return make_pair(shared_ptr<Command>(), CommandId()); // Get the command-id - id = ps.getInt32("id"); + CommandId id = ps.getInt32("id"); // Retrieve the command parameters associated with the received command. ostringstream query; @@ -173,13 +165,12 @@ namespace LOFAR ps.adoptBuffer(buf, "Step." + name + "."); // Create a new BBSStep and return it. - command = BBSStep::create(name, ps, 0); - return make_pair(command, id); + return make_pair(BBSStep::create(name, ps, 0), id); } catch (APSException&) { // In the catch clause we handle all other commands. They can be // constructed using the CommandFactory and initialized using read(). - command = CommandFactory::instance().create(type); + shared_ptr<Command> command(CommandFactory::instance().create(type)); ASSERTSTR(command, "Failed to create a `" << type << "' command object"); command->read(ps); diff --git a/CEP/BB/BBSControl/test/tBBSStrategy.cc b/CEP/BB/BBSControl/test/tBBSStrategy.cc index 5bf75b4a32841a5a48a25c105fc02f94c44b8181..40360adca5ed404609f52aee5ab2524f09bef6fa 100644 --- a/CEP/BB/BBSControl/test/tBBSStrategy.cc +++ b/CEP/BB/BBSControl/test/tBBSStrategy.cc @@ -57,6 +57,10 @@ int main() BBSStrategy writtenStrategy(ps); writtenStrategy.shouldWriteSteps(true); + { + BBSStrategy dummy(writtenStrategy); + } + ps.clear(); cout << writtenStrategy << endl; ps << writtenStrategy; diff --git a/CEP/BB/BBSControl/test/tCommandQueue.cc b/CEP/BB/BBSControl/test/tCommandQueue.cc index 6be89773a6f295e86187911d444a28f966001056..2592f6d51e434f00bb1c71a5e82c7166ecafd2fd 100644 --- a/CEP/BB/BBSControl/test/tCommandQueue.cc +++ b/CEP/BB/BBSControl/test/tCommandQueue.cc @@ -65,7 +65,7 @@ int main(int /*argc*/, char* argv[]) // CommandQueue queue(getenv("USER")); CommandQueue queue("bbs"); CommandQueue::Trigger insert_trig(queue, CommandQueue::Trigger::Command); - vector<const BBSStep*> steps = strategy.getAllSteps(); + vector< shared_ptr<const BBSStep> > steps = strategy.getAllSteps(); ofstream ofs; // Create data; store into command queue and write to reference file.