You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafodion.apache.org by se...@apache.org on 2018/02/20 16:59:44 UTC
[1/2] trafodion git commit: [TRAFODION-2727] Memory leak in the
compiler part of the code in Trafodion
Repository: trafodion
Updated Branches:
refs/heads/master 7c0ab57e7 -> d7644b9dc
[TRAFODION-2727] Memory leak in the compiler part of the code in Trafodion
It was found that CmpMessageReplyCode was not getting deleted at all times.
But, the handling of CmpMessageDescribe messages in the compiler code has some
mysterious issue that corrupts the memory causing unpredictable behavior when
CmpMessageReplyCode is deleted. To circumvent this problem, the CmpStatement heap
is used for embedded compiler to allocate CmpMessageReplyCode and the Context heap
is used in case of standalone compiler. The handling of CmpMessageDescribe will
continue to leak memory in standalone compiler.
Project: http://git-wip-us.apache.org/repos/asf/trafodion/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafodion/commit/52703c64
Tree: http://git-wip-us.apache.org/repos/asf/trafodion/tree/52703c64
Diff: http://git-wip-us.apache.org/repos/asf/trafodion/diff/52703c64
Branch: refs/heads/master
Commit: 52703c64dbca6af7f7ad17d7e0efe15d7f6afd0c
Parents: 087af70
Author: selvaganesang <se...@esgyn.com>
Authored: Fri Feb 16 02:03:46 2018 +0000
Committer: selvaganesang <se...@esgyn.com>
Committed: Fri Feb 16 02:03:46 2018 +0000
----------------------------------------------------------------------
core/sql/arkcmp/CmpConnection.cpp | 8 +++---
core/sql/arkcmp/CmpStatement.cpp | 49 +++++++++++++++++-----------------
core/sql/arkcmp/CmpStatement.h | 8 ++++--
core/sql/optimizer/Rule.cpp | 2 ++
4 files changed, 37 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafodion/blob/52703c64/core/sql/arkcmp/CmpConnection.cpp
----------------------------------------------------------------------
diff --git a/core/sql/arkcmp/CmpConnection.cpp b/core/sql/arkcmp/CmpConnection.cpp
index 0acf0d8..aa72355 100644
--- a/core/sql/arkcmp/CmpConnection.cpp
+++ b/core/sql/arkcmp/CmpConnection.cpp
@@ -349,7 +349,7 @@ void ExCmpMessage::actOnReceive(IpcConnection* )
// The number of NAMemory objects that reside in SYSTEM_MEMORY
// is currently restricted to one at a time--see explanation in
// NAMemory.h.
- cmpStatement = new CTXTHEAP CmpStatement(cmpContext_, 0, NAMemory::SYSTEM_MEMORY);
+ cmpStatement = new CTXTHEAP CmpStatement(cmpContext_);
CmpMessageSQLText sqltext(NULL,0,CTXTHEAP,SQLCHARSETCODE_UNKNOWN,
(CmpMessageObj::MessageTypeEnum)typ);
receiveAndSetUp(this, sqltext);
@@ -388,7 +388,7 @@ void ExCmpMessage::actOnReceive(IpcConnection* )
case (CmpMessageObj::DDL) :
{
- cmpStatement = new CTXTHEAP CmpStatement(cmpContext_, 0, NAMemory::SYSTEM_MEMORY);
+ cmpStatement = new CTXTHEAP CmpStatement(cmpContext_);
CmpMessageDDL statement(NULL, 0, CTXTHEAP);
receiveAndSetUp(this, statement);
cmpStatement->process(statement);
@@ -397,7 +397,7 @@ void ExCmpMessage::actOnReceive(IpcConnection* )
case (CmpMessageObj::DESCRIBE) :
{
- cmpStatement = new CTXTHEAP CmpStatement(cmpContext_, 0, NAMemory::SYSTEM_MEMORY);
+ cmpStatement = new CTXTHEAP CmpStatement(cmpContext_);
CmpMessageDescribe statement(NULL, 0, CTXTHEAP);
receiveAndSetUp(this, statement);
cmpStatement->process(statement);
@@ -433,7 +433,7 @@ void ExCmpMessage::actOnReceive(IpcConnection* )
case (CmpMessageObj::DDL_WITH_STATUS) :
{
- cmpStatement = new CTXTHEAP CmpStatement(cmpContext_, 0, NAMemory::SYSTEM_MEMORY);
+ cmpStatement = new CTXTHEAP CmpStatement(cmpContext_);
CmpMessageDDLwithStatus statement(NULL, 0, CTXTHEAP);
receiveAndSetUp(this, statement);
cmpStatement->process(statement);
http://git-wip-us.apache.org/repos/asf/trafodion/blob/52703c64/core/sql/arkcmp/CmpStatement.cpp
----------------------------------------------------------------------
diff --git a/core/sql/arkcmp/CmpStatement.cpp b/core/sql/arkcmp/CmpStatement.cpp
index 69e1f45..f66baae 100644
--- a/core/sql/arkcmp/CmpStatement.cpp
+++ b/core/sql/arkcmp/CmpStatement.cpp
@@ -136,12 +136,12 @@ CmpStatement::error(Lng32 no, const char* s)
CmpStatement::CmpStatement(CmpContext* context,
- CollHeap* outHeap,
- NAMemory::NAMemoryType memoryType)
+ CollHeap* outHeap)
: parserStmtLiteralList_(outHeap)
{
exceptionRaised_ = FALSE;
- reply_ = 0;
+ reply_ = NULL;
+ bound_ = NULL;
context_ = context;
storedProc_ = 0;
prvCmpStatement_ = 0;
@@ -171,15 +171,17 @@ CmpStatement::CmpStatement(CmpContext* context,
memLimit);
heap_->setErrorCallback(&CmpErrLog::CmpErrLogCallback);
}
+
+ // Embedded arkcmp reply is consumed by the caller before the CmpStatement
+ // is deleted, hence use CmpStatement Heap itself to avoid any leaks
- // The default output heap will be the context heap, because the
- // CmpMessageReply object might last after the CmpStatement goes
- // out of scope.
- outHeap_ = outHeap ? outHeap : context_ ? context_->heap() : 0;
+ if (context_->isEmbeddedArkcmp())
+ outHeap_ = heap_;
+ else
+ outHeap_ = context_->heap();
context->setStatement(this);
-
compStats_ = new (heap_) CompilationStats();
optGlobals_ = new (heap_) OptGlobals(heap_);
@@ -210,9 +212,19 @@ CmpStatement::~CmpStatement()
// to end the interface.
delete storedProc_;
- if (reply_)
+ if (reply_ != NULL)
reply_->decrRefCount();
+/*
+ // At times, this delete can cause corruption in the heap
+ // Hence, it is commented out for now - Selva
+ // To miminze the leak from this the heap_ that used for this
+ // objects comes from CmpStatement Heap in case of embedded arkcmp,
+ // and from CmpContext Heap in case of standalone arkcmp.
+ if (bound_ != NULL)
+ bound_->decrRefCount();
+*/
+
// GLOBAL_EMPTY_INPUT_LOGPROP points to an EstLogProp object in the heap.
// Because it is a SharedPtr, it must be set to NULL before the statement
// heap is destroyed. However, there are cases where a temporary
@@ -760,9 +772,6 @@ CmpStatement::process (const CmpMessageDDL& statement)
QueryText qText(sqlStr, inputCS);
- CmpMessageReplyCode
- *bound = new(outHeap_) CmpMessageReplyCode(outHeap_, statement.id(), 0, 0, outHeap_);
-
// CmpMain cmpmain;
Set_SqlParser_Flags(DELAYED_RESET); // sqlcompCleanup resets for us
Parser parser(CmpCommon::context());
@@ -862,9 +871,6 @@ short CmpStatement::getDDLExprAndNode(char * sqlStr, Lng32 inputCS,
QueryText qText(sqlStr, inputCS);
- // CmpMessageReplyCode
- // *bound = new(outHeap_) CmpMessageReplyCode(outHeap_, statement.id(), 0, 0, outHeap_);
-
Set_SqlParser_Flags(DELAYED_RESET); // sqlcompCleanup resets for us
Parser parser(CmpCommon::context());
BindWA bindWA(ActiveSchemaDB(), CmpCommon::context(), TRUE);
@@ -1033,13 +1039,8 @@ CmpStatement::ReturnStatus
CmpStatement::process (const CmpMessageDescribe& statement)
{
ReturnStatus ret = CmpStatement_SUCCESS;
- // There will be memory leak handling CmpMessageReplyCode *bound this way.
- // The correct way should be making bound a local variable with statementHeap passed in.
- // But Matt has tried it, there seem to be some memory problems show up later,
- // so in the future, when time allows in the memory cleanup stage, this code should be
- // revisited.
- CmpMessageReplyCode
- *bound = new(outHeap_) CmpMessageReplyCode(outHeap_, statement.id(), 0, 0, outHeap_);
+
+ bound_ = new(outHeap_) CmpMessageReplyCode(outHeap_, statement.id(), 0, 0, outHeap_);
reply_ = new(outHeap_) CmpMessageReplyCode(outHeap_, statement.id(), 0, 0, outHeap_);
// A pointer to user SQL query is stored in CmpStatement; if an exception is
@@ -1067,11 +1068,11 @@ CmpStatement::process (const CmpMessageDescribe& statement)
// pass this (casting to RelExpr, which it really is) to CmpDescribe
CmpMain cmpmain;
if (cmpmain.sqlcomp(qText, 0, //IN
- &bound->data(), &bound->size(), bound->outHeap(), //OUT
+ &bound_->data(), &bound_->size(), bound_->outHeap(), //OUT
CmpMain::BIND) //IN
||
CmpDescribe(statement.data(), //IN
- (RelExpr*)bound->data(), //IN
+ (RelExpr*)bound_->data(), //IN
reply_->data(), reply_->size(), reply_->outHeap())) //OUT
{
error(arkcmpErrorNoDiags, statement.data());
http://git-wip-us.apache.org/repos/asf/trafodion/blob/52703c64/core/sql/arkcmp/CmpStatement.h
----------------------------------------------------------------------
diff --git a/core/sql/arkcmp/CmpStatement.h b/core/sql/arkcmp/CmpStatement.h
index dc0767a..097d0cf 100644
--- a/core/sql/arkcmp/CmpStatement.h
+++ b/core/sql/arkcmp/CmpStatement.h
@@ -86,8 +86,7 @@ public:
// memoryType parameter is ignored
CmpStatement(
CmpContext*,
- NAMemory* outHeap = 0,
- NAMemory::NAMemoryType memoryType=NAMemory::DERIVED_MEMORY);
+ NAMemory *outHeap = NULL);
// requests process
ReturnStatus process(const CmpMessageObj&);
@@ -254,6 +253,11 @@ protected:
// The reply to be sent back to executor after processing the request in CmpStatement
CmpMessageReply* reply_;
+ // The result of Compile for statements like invoke, get tables, show stats
+ // that calls CmpDescribe internally immediately after compilation.
+
+ CmpMessageReply *bound_;
+
// The flag to record whether exception has been raised in the
// statement compilation/execution. This is used to clean up properly once the
// exception is raised ( especially when longjmp occurred )
http://git-wip-us.apache.org/repos/asf/trafodion/blob/52703c64/core/sql/optimizer/Rule.cpp
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/Rule.cpp b/core/sql/optimizer/Rule.cpp
index d418fd3..0be2941 100644
--- a/core/sql/optimizer/Rule.cpp
+++ b/core/sql/optimizer/Rule.cpp
@@ -394,6 +394,8 @@ RuleSet::~RuleSet()
{
for (Lng32 i = 0; i < (Lng32)allRules_.entries(); i++)
delete allRules_[i];
+ for (Lng32 i = 0; i < (Lng32)passNRules_.entries(); i++)
+ delete passNRules_[i];
}
void RuleSet::insert(Rule * r)
[2/2] trafodion git commit: Merge PR 1447 [TRAFODION-2727] Memory
leak in the compiler part of the code in Trafodion
Posted by se...@apache.org.
Merge PR 1447 [TRAFODION-2727] Memory leak in the compiler part of the code in Trafodion
Project: http://git-wip-us.apache.org/repos/asf/trafodion/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafodion/commit/d7644b9d
Tree: http://git-wip-us.apache.org/repos/asf/trafodion/tree/d7644b9d
Diff: http://git-wip-us.apache.org/repos/asf/trafodion/diff/d7644b9d
Branch: refs/heads/master
Commit: d7644b9dc8b2cdf90b111ab2f0f5746b550a09f8
Parents: 7c0ab57 52703c6
Author: selvaganesang <se...@apache.org>
Authored: Tue Feb 20 16:58:57 2018 +0000
Committer: selvaganesang <se...@apache.org>
Committed: Tue Feb 20 16:58:57 2018 +0000
----------------------------------------------------------------------
core/sql/arkcmp/CmpConnection.cpp | 8 +++---
core/sql/arkcmp/CmpStatement.cpp | 49 +++++++++++++++++-----------------
core/sql/arkcmp/CmpStatement.h | 8 ++++--
core/sql/optimizer/Rule.cpp | 2 ++
4 files changed, 37 insertions(+), 30 deletions(-)
----------------------------------------------------------------------