Skip to content

Commit

Permalink
Refs #4554 make sure all destructors are virtual and ctors are called
Browse files Browse the repository at this point in the history
To fix segfaults on destruction. Multiple virtual inheritance is hard ;)
  • Loading branch information
Janik Zikovsky committed Jan 20, 2012
1 parent be86fc7 commit 272de26
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 90 deletions.
3 changes: 1 addition & 2 deletions Code/Mantid/Framework/API/inc/MantidAPI/IMDWorkspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ namespace Mantid
public:
IMDWorkspace();
IMDWorkspace(const IMDWorkspace & other);
virtual ~IMDWorkspace();

/// Get the number of points associated with the workspace.
/// For MDEvenWorkspace it is the number of events contributing into the workspace
Expand All @@ -85,8 +86,6 @@ namespace Mantid
return this->getSignalAtCoord(coords.getBareArray());
}

virtual ~IMDWorkspace();

};

/// Shared pointer to the IMDWorkspace base class
Expand Down
2 changes: 1 addition & 1 deletion Code/Mantid/Framework/API/inc/MantidAPI/MDGeometry.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ namespace API
public:
MDGeometry();
MDGeometry(const MDGeometry & other);
~MDGeometry();
virtual ~MDGeometry();
void initGeometry(std::vector<Mantid::Geometry::IMDDimension_sptr> & dimensions);

// --------------------------------------------------------------------------------------------
Expand Down
2 changes: 2 additions & 0 deletions Code/Mantid/Framework/API/inc/MantidAPI/Workspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class MANTID_API_DLL Workspace : public Kernel::DataItem
{
public:
Workspace();
virtual ~Workspace();

// DataItem interface
/// Name
virtual const std::string name() const { return this->getName(); }
Expand Down
129 changes: 51 additions & 78 deletions Code/Mantid/Framework/API/src/Algorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,6 @@ namespace Mantid

}// end of for loop for checking the properties for workspace groups


throw;

// Invoke exec() method of derived class and catch all uncaught exceptions
try
{
Expand Down Expand Up @@ -360,23 +357,16 @@ namespace Mantid

catch (...)
{
// Gaudi sets the executed flag to true here despite the exception
// This allows it to move to the next command or it just loops indefinitely.
// we will set it to false (see Nick Draper) 6/12/07
// Execution failed
setExecuted(false);
m_runningAsync = false;
m_running = false;

m_notificationCenter.postNotification(new ErrorNotification(this,"UNKNOWN Exception is caught "));
g_log.error() << this->name() << ": UNKNOWN Exception is caught\n";
throw;
// Gaudi calls exception service 'handle' method here
}

// Gaudi has some stuff here where it tests for failure, increments the error counter
// and then converts to success if less than the maximum. This is clearly related to
// having an event loop, and thus we shouldn't want it. This is the only place it's used.

m_notificationCenter.postNotification(new FinishedNotification(this,isExecuted()));
// Only gets to here if algorithm ended normally
// Free up any memory available.
Expand Down Expand Up @@ -809,30 +799,30 @@ namespace Mantid
}

/** To Process workspace groups.
* @param ingrpws_sptr :: input workspacegroup pointer to iterate through all members
* @param props a vector holding the input properties
* @returns true - if all the workspace members are executed.
*/
bool Algorithm::processGroups(WorkspaceGroup_sptr ingrpws_sptr,const std::vector<Mantid::Kernel::Property*>& props)
* This runs the algorithm once for each workspace in the group.
*
* @param ingrpws_sptr :: input workspacegroup pointer to iterate through all members
* @param props :: a vector holding the input properties
* @return true - if all the workspace members are executed.
*/
bool Algorithm::processGroups(WorkspaceGroup_sptr ingrpws_sptr, const std::vector<Mantid::Kernel::Property*>& props)
{
int nPeriod=1;
int execPercentage=0;
bool bgroupPassed=true;

WorkspaceGroup_sptr wsgrp1_sptr;
WorkspaceGroup_sptr wsgrp2_sptr;
bool bnewGoup1=true;
bool bnewGoup2=true;
// Vector of each Output WorkspaceProperty, that now becomes a group
std::vector<WorkspaceGroup_sptr> outputGroups;
std::string prevPropName("");

//getting the input workspace group names
const std::vector<std::string> inputWSNames=ingrpws_sptr->getNames();
const size_t nSize=inputWSNames.size();
//size is one if only group header.
//return if atleast one meber is not there in group to process
// Size is one if only group header.
// There needs to be at least one member in the input group
if(nSize<1)
{ throw std::runtime_error("Input WorkspaceGroup has no members to process");
}
throw std::runtime_error("Input WorkspaceGroup has no members to process");


int execTotal=0;
//removing the header count from the totalsize
Expand All @@ -848,95 +838,81 @@ namespace Mantid
std::string ingrpwsName;
getInputGroupWorkspaceName(props,ingrpwsName);
//check the workspace are of similar names (similar if they are of names like group_1,group_2)
bool bSimilarNames=isGroupWorkspacesofSimilarNames(ingrpwsName,inputWSNames);
bool bSimilarNames = isGroupWorkspacesofSimilarNames(ingrpwsName,inputWSNames);

// For each workspace in the group
std::vector<std::string>::const_iterator wsItr=inputWSNames.begin();
for(;wsItr!=inputWSNames.end();++wsItr)
{ //set properties

{
// Set properties
if (API::AnalysisDataService::Instance().retrieve(*wsItr)->id() != "TableWorkspace")
{
// Do the rest of the for loop
std::vector<Mantid::Kernel::Property*>::const_iterator itr;
for (itr=props.begin();itr!=props.end();++itr)
{
// cppcheck-suppress variableScope
int outWSCount=0;
size_t outWSCount = 0;
if(isWorkspaceProperty(*itr) )
{

if(isInputWorkspaceProperty(*itr))
{

// Replace the input workspace property with the name of the WS
// that is in the group
if(!setInputWSProperties(alg,*itr,*wsItr))
{
throw std::runtime_error("Invalid value found for the property " +(*itr)->name()+ " when executing the algorithm"+this->name());
}

}
if(isOutputWorkspaceProperty(*itr))
{
++outWSCount;
//create a group and pass that to setOutputWSProperties
if(outWSCount==1)
{
if( bnewGoup1)
{
wsgrp1_sptr= WorkspaceGroup_sptr(new WorkspaceGroup);
bnewGoup1=false;
}
if(!setOutputWSProperties(alg,*itr,nPeriod,*wsItr,wsgrp1_sptr,bSimilarNames,bequal))
{
throw std::runtime_error("Invalid value found for the property " +(*itr)->name()+ " when executing the algorithm"+this->name());
}

}
if(outWSCount==2)
if (outWSCount > outputGroups.size())
{
if( bnewGoup2)
{
wsgrp2_sptr= WorkspaceGroup_sptr(new WorkspaceGroup);
bnewGoup2=false;
}
if(!setOutputWSProperties(alg,*itr,nPeriod,*wsItr,wsgrp2_sptr,bSimilarNames,bequal))
{
throw std::runtime_error("Invalid value found for the property " +(*itr)->name()+ " when executing the algorithm"+this->name());
}
// Create a new OUTPUT group when needed
outputGroups.push_back(WorkspaceGroup_sptr(new WorkspaceGroup));
}

// This is the output group workspace for this property.
WorkspaceGroup_sptr thisOutputGroup = outputGroups[outWSCount-1];

// Set the workspace
if(!setOutputWSProperties(alg,*itr,nPeriod,*wsItr,thisOutputGroup,bSimilarNames,bequal))
throw std::runtime_error("Invalid value found for the property " +(*itr)->name()+ " when executing the algorithm"+this->name());

}//end of isOutputWorkspaceProperty

}// end of isWorkspaceProperty
else
{
// Simply copy any other property (by string)
this->setOtherProperties(alg,(*itr)->name(),(*itr)->value(),nPeriod);
}
}//end of for loop for setting properties

//resetting the previous properties at the end of each execution
prevPropName="";

// execute the algorithm
bool bStatus = false;
if ( alg->validateProperties() )
{bStatus = alg->execute();
}
bStatus = alg->execute();

// status of each execution is checking
bgroupPassed=bgroupPassed&&bStatus;
execPercentage+=10;
bgroupPassed = bgroupPassed && bStatus;
execPercentage += 10;
progress(double((execPercentage)/execTotal));

//if a workspace execution fails
if(!bStatus)
{
throw std::runtime_error("execution failed for the input workspace "+(*wsItr));
}
throw std::runtime_error("Execution failed for the input workspace "+(*wsItr));
}
//increment count for outworkpsace name
nPeriod++;

}//end of for loop for input workspace group members processing

//if all passed
if(bgroupPassed)
{setExecuted(true);
}
setExecuted(true);

m_notificationCenter.postNotification(new FinishedNotification(this,isExecuted()));
return bgroupPassed;
Expand Down Expand Up @@ -1044,12 +1020,12 @@ namespace Mantid
}

/** Setting input workspace properties for an algorithm,for handling workspace groups.
* @param pAlg :: pointer to algorithm
* @param prop pointer to a vector holding the input properties
* @param inMemberWSName input workspace name
* @param pAlg :: pointer to algorithm
* @param prop :: property to set
* @param inMemberWSName :: input workspace name
* @returns true - if property is set .
*/
bool Algorithm::setInputWSProperties(IAlgorithm* pAlg,Mantid::Kernel::Property* prop,const std::string& inMemberWSName )
bool Algorithm::setInputWSProperties(IAlgorithm* pAlg, Mantid::Kernel::Property* prop,const std::string& inMemberWSName )
{
if(!pAlg) return false;
if(!prop)return false;
Expand All @@ -1064,9 +1040,10 @@ namespace Mantid
return true;

}
/** setting output workspace properties for an algorithm,
* This used for processing workspace groups and this method is called when
* the input and out workspaces are not same
/** Setting output workspace properties for an algorithm,
* This used for processing workspace groups and this method is called when
* the input and out workspaces are not same.
*
* @param pAlg :: pointer to algorithm
* @param prop :: pointer to the input properties
* @param nPeriod :: period number
Expand Down Expand Up @@ -1100,14 +1077,10 @@ namespace Mantid
if(bSimilarNames) //if group are of similar names
{
outmemberwsName=outgrpwsName+"_"+suffix.str(); // input group ws ="Group" ,//OutPut GroupWs ="NewGroup"
//input member ws1="Group_1",//output member ws1="NewGroup_1"
//input member ws2="Group_2",//output member ws2="NewGroup_2"
}
else
{
outmemberwsName= inmemberwsName+"_"+outgrpwsName;//InputPut GroupWs ="Group",//OutPut GroupWs ="NewGroup"
// input member ws1 = "Bob", // output member ws1="Bob_NewGroup"
//input member ws2 = "Sally",// output member ws2="Sally_NewGroup"
}
}
if(nPeriod==1)
Expand Down
1 change: 1 addition & 0 deletions Code/Mantid/Framework/API/src/IMDHistoWorkspace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace API
/** Constructor
*/
IMDHistoWorkspace::IMDHistoWorkspace()
: IMDWorkspace(), MultipleExperimentInfos()
{
}

Expand Down
8 changes: 4 additions & 4 deletions Code/Mantid/Framework/API/src/IMDWorkspace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ namespace Mantid
{
namespace API
{
IMDWorkspace::~IMDWorkspace()
{
}

//-----------------------------------------------------------------------------------------------
/** Default constructor */
IMDWorkspace::IMDWorkspace()
Expand All @@ -26,6 +22,10 @@ namespace Mantid
{
}

/// Destructor
IMDWorkspace::~IMDWorkspace()
{
}

/**
* Default implementation throws NotImplementedError exception.
Expand Down
1 change: 1 addition & 0 deletions Code/Mantid/Framework/API/src/MDGeometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ namespace API
// Stop watching once object is deleted
API::AnalysisDataService::Instance().notificationCenter.removeObserver(m_delete_observer);
}
m_dimensions.clear();
}


Expand Down
10 changes: 9 additions & 1 deletion Code/Mantid/Framework/API/src/Workspace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,17 @@ namespace API
{

/// Default constructor
Workspace::Workspace() : m_title(), m_comment(), m_name(), m_history()
Workspace::Workspace()
: DataItem(),
m_title(), m_comment(), m_name(), m_history()
{}

/// Workspace destructor
Workspace::~Workspace()
{
}


/** Set the title of the workspace
*
* @param t :: The title
Expand Down
2 changes: 1 addition & 1 deletion Code/Mantid/Framework/Kernel/inc/MantidKernel/DataItem.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace Mantid
class MANTID_KERNEL_DLL DataItem
{
public:
/// Default constructore
/// Default constructor
DataItem();
/// Destructor
virtual ~DataItem();
Expand Down
3 changes: 2 additions & 1 deletion Code/Mantid/Framework/Kernel/src/DataItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ namespace Mantid
*/
DataItem::~DataItem()
{
delete m_lock;
// delete m_lock;
// m_lock = NULL;
}

/** Private method to access the RWLock object.
Expand Down
6 changes: 4 additions & 2 deletions Code/Mantid/Framework/MDEvents/src/MDHistoWorkspace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ namespace MDEvents
*/
MDHistoWorkspace::MDHistoWorkspace(Mantid::Geometry::MDHistoDimension_sptr dimX, Mantid::Geometry::MDHistoDimension_sptr dimY,
Mantid::Geometry::MDHistoDimension_sptr dimZ, Mantid::Geometry::MDHistoDimension_sptr dimT)
: numDimensions(0)
: IMDHistoWorkspace(),
numDimensions(0)
{
std::vector<Mantid::Geometry::MDHistoDimension_sptr> dimensions;
if (dimX) dimensions.push_back(dimX);
Expand All @@ -38,7 +39,8 @@ namespace MDEvents
* @param dimensions :: vector of MDHistoDimension; no limit to how many.
*/
MDHistoWorkspace::MDHistoWorkspace(std::vector<Mantid::Geometry::MDHistoDimension_sptr> & dimensions)
: numDimensions(0)
: IMDHistoWorkspace(),
numDimensions(0)
{
this->init(dimensions);
}
Expand Down

0 comments on commit 272de26

Please sign in to comment.