1
0
mirror of https://github.com/mariadb-corporation/mariadb-columnstore-engine.git synced 2025-07-30 19:23:07 +03:00

MCOL-4043 Fix memory leaks - 1 (second attempt)

simpleScalarFilterToParseTree() performs a dynamic allocation
of a ParseTree object, but this memory is never freed later.
We now keep track of this allocation and perform the delete
in ~CSEP/CSEP::unserialize() after the query finishes.
This commit is contained in:
Gagan Goel
2020-06-19 15:39:49 -04:00
parent 6d5053e651
commit b48cf64b78
8 changed files with 75 additions and 2 deletions

View File

@ -173,6 +173,26 @@ CalpontSelectExecutionPlan::~CalpontSelectExecutionPlan()
fFilters = NULL; fFilters = NULL;
fHaving = NULL; fHaving = NULL;
if (!fDynamicParseTreeVec.empty())
{
for (auto& parseTree : fDynamicParseTreeVec)
{
if (parseTree)
{
// 'delete fFilters;' above has already deleted objects pointed
// to by parseTree->left()/right()/data(), so we set the
// pointers to NULL here before calling 'delete parseTree;'
parseTree->left((ParseTree*) (NULL));
parseTree->right((ParseTree*) (NULL));
parseTree->data((TreeNode*) (NULL));
delete parseTree;
parseTree = NULL;
}
}
fDynamicParseTreeVec.clear();
}
} }
/** /**
@ -537,6 +557,26 @@ void CalpontSelectExecutionPlan::unserialize(messageqcpp::ByteStream& b)
fHaving = 0; fHaving = 0;
} }
if (!fDynamicParseTreeVec.empty())
{
for (auto& parseTree : fDynamicParseTreeVec)
{
if (parseTree)
{
// 'delete fFilters;' above has already deleted objects pointed
// to by parseTree->left()/right()/data(), so we set the
// pointers to NULL here before calling 'delete parseTree;'
parseTree->left((ParseTree*) (NULL));
parseTree->right((ParseTree*) (NULL));
parseTree->data((TreeNode*) (NULL));
delete parseTree;
parseTree = NULL;
}
}
fDynamicParseTreeVec.clear();
}
messageqcpp::ByteStream::quadbyte size; messageqcpp::ByteStream::quadbyte size;
messageqcpp::ByteStream::quadbyte i; messageqcpp::ByteStream::quadbyte i;

View File

@ -772,6 +772,12 @@ public:
return ((fSessionID & 0x80000000) != 0); return ((fSessionID & 0x80000000) != 0);
} }
virtual void setDynamicParseTreeVec(
const std::vector<execplan::ParseTree*>& dynamicParseTreeVec)
{
fDynamicParseTreeVec = dynamicParseTreeVec;
}
/** /**
* Protected stuff * Protected stuff
*/ */
@ -922,6 +928,8 @@ private:
bool fIsDML; bool fIsDML;
std::string fTimeZone; std::string fTimeZone;
std::vector<execplan::ParseTree*> fDynamicParseTreeVec;
}; };
/** /**

View File

@ -150,8 +150,14 @@ SimpleFilter::SimpleFilter(const SimpleFilter& rhs) :
SimpleFilter::~SimpleFilter() SimpleFilter::~SimpleFilter()
{ {
//delete fOp; //delete fOp;
delete fLhs; if (fLhs != NULL)
delete fRhs; delete fLhs;
if (fRhs != NULL)
delete fRhs;
fLhs = NULL;
fRhs = NULL;
} }
/** /**

View File

@ -364,6 +364,11 @@ struct JobInfo
bool isDML; bool isDML;
std::string timeZone; std::string timeZone;
// This is for tracking any dynamically allocated ParseTree objects
// in simpleScalarFilterToParseTree() for later deletion in
// ~csep() or csep.unserialize()
std::vector<execplan::ParseTree*> dynamicParseTreeVec;
private: private:
//defaults okay //defaults okay
//JobInfo(const JobInfo& rhs); //JobInfo(const JobInfo& rhs);

View File

@ -3269,6 +3269,12 @@ void doOR(ParseTree* n, JobInfo& jobInfo, bool tryCombine)
ccp->left(parseTree->left()); ccp->left(parseTree->left());
ccp->right(parseTree->right()); ccp->right(parseTree->right());
ccp->data(parseTree->data()); ccp->data(parseTree->data());
jobInfo.dynamicParseTreeVec.push_back(parseTree);
}
else if (parseTree)
{
delete parseTree;
parseTree = NULL;
} }
} }

View File

@ -192,7 +192,10 @@ void ssfInHaving(ParseTree* pt, void* obj)
pt->right(parseTree->right()); pt->right(parseTree->right());
pt->data(parseTree->data()); pt->data(parseTree->data());
jobInfo->dynamicParseTreeVec.push_back(parseTree);
// don't delete the parseTree, it has been placed in the plan. // don't delete the parseTree, it has been placed in the plan.
// Instead, we use the dynamicParseTreeVec above for deletion
// in ~csep() or csep.unserialize().
// delete parseTree; // delete parseTree;
} }
else else
@ -627,7 +630,10 @@ void doSimpleScalarFilter(ParseTree* p, JobInfo& jobInfo)
// create job steps for each simple filter // create job steps for each simple filter
JLF_ExecPlanToJobList::walkTree(parseTree, jobInfo); JLF_ExecPlanToJobList::walkTree(parseTree, jobInfo);
jobInfo.dynamicParseTreeVec.push_back(parseTree);
// don't delete the parseTree, it has been placed in the plan. // don't delete the parseTree, it has been placed in the plan.
// Instead, we use the dynamicParseTreeVec above for deletion
// in ~csep() or csep.unserialize().
// delete parseTree; // delete parseTree;
} }
else else

View File

@ -2003,6 +2003,7 @@ SJLP makeJobList_(
jl->addQuery(querySteps); jl->addQuery(querySteps);
jl->addProject(projectSteps); jl->addProject(projectSteps);
jl->addDelivery(deliverySteps); jl->addDelivery(deliverySteps);
csep->setDynamicParseTreeVec(jobInfo.dynamicParseTreeVec);
dynamic_cast<TupleJobList*>(jl)->setDeliveryFlag(true); dynamic_cast<TupleJobList*>(jl)->setDeliveryFlag(true);
} }

View File

@ -169,6 +169,7 @@ SJSTEP& SubQueryTransformer::makeSubQueryStep(execplan::CalpontSelectExecutionPl
fSubJobList->addQuery(querySteps); fSubJobList->addQuery(querySteps);
fSubJobList->addDelivery(deliverySteps); fSubJobList->addDelivery(deliverySteps);
fSubJobList->putEngineComm(DistributedEngineComm::instance(fOutJobInfo->rm)); fSubJobList->putEngineComm(DistributedEngineComm::instance(fOutJobInfo->rm));
csep->setDynamicParseTreeVec(fSubJobInfo->dynamicParseTreeVec);
// Get the correlated steps // Get the correlated steps
fCorrelatedSteps = fSubJobInfo->correlateSteps; fCorrelatedSteps = fSubJobInfo->correlateSteps;