You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafodion.apache.org by "Zhu, Wen-Jun" <we...@esgyn.cn> on 2018/08/14 11:10:51 UTC
refCount on ComDiagsArea
hi,
When setting a ComDiagsArea, I find the refCount did not increase, in function atp_struct::setDiagsArea of file core/sql/exp/ExpAtp.h:
inline void atp_struct::setDiagsArea(ComDiagsArea* diagsArea)
{
if (diagsArea_)
diagsArea_->decrRefCount();
diagsArea_ = diagsArea;
}
I guess this is a problem.
There are two solutions to fix this:
1. Invoke incrRefCount for ComDiagsArea to increase, just after the assignment.
2. Overload the operator= for ComDiagsArea, to increase within the operator=,
I find operator= is declared in ComDiags.h, but there is no implementation.
The 2nd solution may be better, as the both increment for the left-hand side ComDiagsArea and
the decrement for the right-hand side ComDiagsAre can be handled within a single operator=,
which is friendly to users, like shared_ptr in C++.
Regards,
Wenjun Zhu
RE: refCount on ComDiagsArea
Posted by Selva Govindarajan <se...@esgyn.com>.
Hi Wen-Jun,
Are you planning for general cleanup or attempting to solve a specific problem? If it is latter, can you please describe the problem? I might be able to help you to resolve it possibly differently. This suggestion is based on the similar fear outlined by Dave's comment earlier.
Below is what I have understood how reference count used in Trafodion.
An object gets a refCount_ only when it is derived from IpcMessageObj. Only the objects that would be shipped between processes need to be derived from IpcMessageObj. ComDiagsArea is one such object. You can look at any object derived from IpcMessageObj to determine how reference count is used.
See the code snippet below
this << *cqReply;
if (!(didAttemptCancel ||cancelByPid))
{
if (diags == NULL)
{
diags = ComDiagsArea::allocate(getHeap());
*diags << DgSqlCode(-EXE_CANCEL_QID_NOT_FOUND);
}
*this << *diags;
}
send(FALSE);
cqReply->decrRefCount();
if (diags)
diags->decrRefCount();
request->decrRefCount();
In the above code, the concatenate operator << increments the reference count. The “send” queues in the request to be sent. When decrRefCount sees reference count as 0, it deallocates the IpcMessageObj. "Send" also calls decrRefCount. Whichever decrRefCount happens last either in "send" or in this function deletes the IpcMessageObj to avoid memory leak. It is also possible if you don't manage reference count correctly, you could end up looking at a deleted object.
Also, in general, in executor layer passing pointers between different object doesn't involve reference count management because refCount_ member variable is available only when the object would be shipped between processes.
You can also consult the comments about reference count in export/IpcMessageObj.h
Selva
-----Original Message-----
From: Qifan Chen <qi...@esgyn.com>
Sent: Wednesday, August 15, 2018 6:13 AM
To: dev@trafodion.apache.org
Subject: Re: refCount on ComDiagsArea
Hi Wen-Jun,
Thanks for the info.
I wonder if you can also provide an example of a ComDiagArea object that is mistakenly deleted (see the two relevant methods below) due to incorrectly maintained reference count.
Thanks --Qifan
383 IpcMessageRefCount ComDiagsArea::decrRefCount()
384 {
385 if (getRefCount() == 1)
386 {
387 deAllocate();
388 return 0;
389 }
390
391 // Let base class do the work.
392 return this->IpcMessageObj::decrRefCount();
393 }
1513 inline
1514 void ComDiagsArea::deAllocate()
1515 {
1516 if (collHeapPtr_ == NULL)
1517 delete this;
1518 else {
1519 // save collHeapPtr, because destroyMe() sets it to NULL
1520 // Better solution: derive ComDiagsArea from NABasicObject and get
1521 // rid of allocate() / deAllocate()
1522 CollHeap * p = collHeapPtr_;
1523 destroyMe();
1524 p->deallocateMemory(this);
1525 };
1526 }
1527
________________________________
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 9:51:40 PM
To: dev@trafodion.apache.org
Subject: 答复: refCount on ComDiagsArea
Hi,
It is true that there are some other functions incrementing the reference count, like in atp_struct::copyAtp:
if (from->getDiagsArea())
from->getDiagsArea()->incrRefCount();
setDiagsArea(from->getDiagsArea());
incrRefCount() is called to increase the ref count.
But when I check the result of command
grep setDiagsArea * -B4 -A4 -rw
I find lots of code have not invoked incrRefCount(), like core/sql/exp/exp_eval.cpp:
479 if (retcode == ex_expr::EXPR_ERROR)
480 {
481 if (diagsArea != atp1->getDiagsArea())
482 atp1->setDiagsArea(diagsArea);
483
484 return retcode;
485 }
There's bug here.
So, how about to wrap operator= invocation in atp_struct::setDiagsArea(), and in operator= we deal with the ref count, like Qifan suggested?
-----邮件原件-----
发件人: Selva Govindarajan <se...@esgyn.com>
发送时间: 2018年8月15日 0:23
收件人: dev@trafodion.apache.org
主题: RE: refCount on ComDiagsArea
I second Dave's comment. I was about to comment in similar lines. When I saw Dave's message, I had no second thoughts to second it.
Selva
-----Original Message-----
From: Qifan Chen <qi...@esgyn.com>
Sent: Tuesday, August 14, 2018 9:11 AM
To: dev@trafodion.apache.org
Subject: Re: refCount on ComDiagsArea
Hi,
I personally also like the 2nd option which is to overload the operator= for ComDiagsArea.
But because of high number of calls to atp_struct::setDiagsArea() in the executor, it is better to hide the call to operator=(ComDiagsArea&) inside atp_struct::setDiagsArea().
Also keep in mind that we can not ubiquitously use C11 structs such as shared_ptr or make_shared in our code base yet since we still need to support platforms such as CentOS6 that do not have C11 support package.
Thanks --Qifan
________________________________
From: Dave Birdsall <da...@esgyn.com>
Sent: Tuesday, August 14, 2018 10:10:43 AM
To: dev@trafodion.apache.org
Subject: RE: refCount on ComDiagsArea
Hi,
I have not researched this area, but it strikes me as one that could be very delicate. It may be that in most code paths it is assumed that some other function is incrementing the reference count. Great care should be taken in modifying this otherwise it may lead to memory leaks. I am hoping others who are more knowledgeable will add to this discussion.
Can you give more insight into what problem led you here?
Dave
-----Original Message-----
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 4:11 AM
To: dev@trafodion.apache.org
Subject: refCount on ComDiagsArea
hi,
When setting a ComDiagsArea, I find the refCount did not increase, in function atp_struct::setDiagsArea of file core/sql/exp/ExpAtp.h:
inline void atp_struct::setDiagsArea(ComDiagsArea* diagsArea) {
if (diagsArea_)
diagsArea_->decrRefCount();
diagsArea_ = diagsArea;
}
I guess this is a problem.
There are two solutions to fix this:
1. Invoke incrRefCount for ComDiagsArea to increase, just after the assignment.
2. Overload the operator= for ComDiagsArea, to increase within the operator=,
I find operator= is declared in ComDiags.h, but there is no implementation.
The 2nd solution may be better, as the both increment for the left-hand side ComDiagsArea and the decrement for the right-hand side ComDiagsAre can be handled within a single operator=, which is friendly to users, like shared_ptr in C++.
Regards,
Wenjun Zhu
Re: refCount on ComDiagsArea
Posted by Qifan Chen <qi...@esgyn.com>.
Hi Wen-Jun,
Thanks for the info.
I wonder if you can also provide an example of a ComDiagArea object that is mistakenly deleted (see the two relevant methods below) due to incorrectly maintained reference count.
Thanks --Qifan
383 IpcMessageRefCount ComDiagsArea::decrRefCount()
384 {
385 if (getRefCount() == 1)
386 {
387 deAllocate();
388 return 0;
389 }
390
391 // Let base class do the work.
392 return this->IpcMessageObj::decrRefCount();
393 }
1513 inline
1514 void ComDiagsArea::deAllocate()
1515 {
1516 if (collHeapPtr_ == NULL)
1517 delete this;
1518 else {
1519 // save collHeapPtr, because destroyMe() sets it to NULL
1520 // Better solution: derive ComDiagsArea from NABasicObject and get
1521 // rid of allocate() / deAllocate()
1522 CollHeap * p = collHeapPtr_;
1523 destroyMe();
1524 p->deallocateMemory(this);
1525 };
1526 }
1527
________________________________
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 9:51:40 PM
To: dev@trafodion.apache.org
Subject: 答复: refCount on ComDiagsArea
Hi,
It is true that there are some other functions incrementing the reference count, like in atp_struct::copyAtp:
if (from->getDiagsArea())
from->getDiagsArea()->incrRefCount();
setDiagsArea(from->getDiagsArea());
incrRefCount() is called to increase the ref count.
But when I check the result of command
grep setDiagsArea * -B4 -A4 -rw
I find lots of code have not invoked incrRefCount(), like core/sql/exp/exp_eval.cpp:
479 if (retcode == ex_expr::EXPR_ERROR)
480 {
481 if (diagsArea != atp1->getDiagsArea())
482 atp1->setDiagsArea(diagsArea);
483
484 return retcode;
485 }
There's bug here.
So, how about to wrap operator= invocation in atp_struct::setDiagsArea(), and in operator= we deal with the ref count, like Qifan suggested?
-----邮件原件-----
发件人: Selva Govindarajan <se...@esgyn.com>
发送时间: 2018年8月15日 0:23
收件人: dev@trafodion.apache.org
主题: RE: refCount on ComDiagsArea
I second Dave's comment. I was about to comment in similar lines. When I saw Dave's message, I had no second thoughts to second it.
Selva
-----Original Message-----
From: Qifan Chen <qi...@esgyn.com>
Sent: Tuesday, August 14, 2018 9:11 AM
To: dev@trafodion.apache.org
Subject: Re: refCount on ComDiagsArea
Hi,
I personally also like the 2nd option which is to overload the operator= for ComDiagsArea.
But because of high number of calls to atp_struct::setDiagsArea() in the executor, it is better to hide the call to operator=(ComDiagsArea&) inside atp_struct::setDiagsArea().
Also keep in mind that we can not ubiquitously use C11 structs such as shared_ptr or make_shared in our code base yet since we still need to support platforms such as CentOS6 that do not have C11 support package.
Thanks --Qifan
________________________________
From: Dave Birdsall <da...@esgyn.com>
Sent: Tuesday, August 14, 2018 10:10:43 AM
To: dev@trafodion.apache.org
Subject: RE: refCount on ComDiagsArea
Hi,
I have not researched this area, but it strikes me as one that could be very delicate. It may be that in most code paths it is assumed that some other function is incrementing the reference count. Great care should be taken in modifying this otherwise it may lead to memory leaks. I am hoping others who are more knowledgeable will add to this discussion.
Can you give more insight into what problem led you here?
Dave
-----Original Message-----
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 4:11 AM
To: dev@trafodion.apache.org
Subject: refCount on ComDiagsArea
hi,
When setting a ComDiagsArea, I find the refCount did not increase, in function atp_struct::setDiagsArea of file core/sql/exp/ExpAtp.h:
inline void atp_struct::setDiagsArea(ComDiagsArea* diagsArea) {
if (diagsArea_)
diagsArea_->decrRefCount();
diagsArea_ = diagsArea;
}
I guess this is a problem.
There are two solutions to fix this:
1. Invoke incrRefCount for ComDiagsArea to increase, just after the assignment.
2. Overload the operator= for ComDiagsArea, to increase within the operator=,
I find operator= is declared in ComDiags.h, but there is no implementation.
The 2nd solution may be better, as the both increment for the left-hand side ComDiagsArea and the decrement for the right-hand side ComDiagsAre can be handled within a single operator=, which is friendly to users, like shared_ptr in C++.
Regards,
Wenjun Zhu
RE: refCount on ComDiagsArea
Posted by Dave Birdsall <da...@esgyn.com>.
Hi,
Give it a try!
One place to check carefully is how it affects ComDiagsArea objects sent over IPC (thanks to Selva for this pointer). When we send an object over IPC, the reference count loses its meaning and has to be re-initialized on the receiving end appropriately. Check that your change doesn't break anything in this area.
Dave
-----Original Message-----
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 7:52 PM
To: dev@trafodion.apache.org
Subject: 答复: refCount on ComDiagsArea
Hi,
It is true that there are some other functions incrementing the reference count, like in atp_struct::copyAtp:
if (from->getDiagsArea())
from->getDiagsArea()->incrRefCount();
setDiagsArea(from->getDiagsArea());
incrRefCount() is called to increase the ref count.
But when I check the result of command
grep setDiagsArea * -B4 -A4 -rw
I find lots of code have not invoked incrRefCount(), like core/sql/exp/exp_eval.cpp:
479 if (retcode == ex_expr::EXPR_ERROR)
480 {
481 if (diagsArea != atp1->getDiagsArea())
482 atp1->setDiagsArea(diagsArea);
483
484 return retcode;
485 }
There's bug here.
So, how about to wrap operator= invocation in atp_struct::setDiagsArea(), and in operator= we deal with the ref count, like Qifan suggested?
-----邮件原件-----
发件人: Selva Govindarajan <se...@esgyn.com>
发送时间: 2018年8月15日 0:23
收件人: dev@trafodion.apache.org
主题: RE: refCount on ComDiagsArea
I second Dave's comment. I was about to comment in similar lines. When I saw Dave's message, I had no second thoughts to second it.
Selva
-----Original Message-----
From: Qifan Chen <qi...@esgyn.com>
Sent: Tuesday, August 14, 2018 9:11 AM
To: dev@trafodion.apache.org
Subject: Re: refCount on ComDiagsArea
Hi,
I personally also like the 2nd option which is to overload the operator= for ComDiagsArea.
But because of high number of calls to atp_struct::setDiagsArea() in the executor, it is better to hide the call to operator=(ComDiagsArea&) inside atp_struct::setDiagsArea().
Also keep in mind that we can not ubiquitously use C11 structs such as shared_ptr or make_shared in our code base yet since we still need to support platforms such as CentOS6 that do not have C11 support package.
Thanks --Qifan
________________________________
From: Dave Birdsall <da...@esgyn.com>
Sent: Tuesday, August 14, 2018 10:10:43 AM
To: dev@trafodion.apache.org
Subject: RE: refCount on ComDiagsArea
Hi,
I have not researched this area, but it strikes me as one that could be very delicate. It may be that in most code paths it is assumed that some other function is incrementing the reference count. Great care should be taken in modifying this otherwise it may lead to memory leaks. I am hoping others who are more knowledgeable will add to this discussion.
Can you give more insight into what problem led you here?
Dave
-----Original Message-----
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 4:11 AM
To: dev@trafodion.apache.org
Subject: refCount on ComDiagsArea
hi,
When setting a ComDiagsArea, I find the refCount did not increase, in function atp_struct::setDiagsArea of file core/sql/exp/ExpAtp.h:
inline void atp_struct::setDiagsArea(ComDiagsArea* diagsArea) {
if (diagsArea_)
diagsArea_->decrRefCount();
diagsArea_ = diagsArea;
}
I guess this is a problem.
There are two solutions to fix this:
1. Invoke incrRefCount for ComDiagsArea to increase, just after the assignment.
2. Overload the operator= for ComDiagsArea, to increase within the operator=,
I find operator= is declared in ComDiags.h, but there is no implementation.
The 2nd solution may be better, as the both increment for the left-hand side ComDiagsArea and the decrement for the right-hand side ComDiagsAre can be handled within a single operator=, which is friendly to users, like shared_ptr in C++.
Regards,
Wenjun Zhu
答复: refCount on ComDiagsArea
Posted by "Zhu, Wen-Jun" <we...@esgyn.cn>.
Hi,
It is true that there are some other functions incrementing the reference count, like in atp_struct::copyAtp:
if (from->getDiagsArea())
from->getDiagsArea()->incrRefCount();
setDiagsArea(from->getDiagsArea());
incrRefCount() is called to increase the ref count.
But when I check the result of command
grep setDiagsArea * -B4 -A4 -rw
I find lots of code have not invoked incrRefCount(), like core/sql/exp/exp_eval.cpp:
479 if (retcode == ex_expr::EXPR_ERROR)
480 {
481 if (diagsArea != atp1->getDiagsArea())
482 atp1->setDiagsArea(diagsArea);
483
484 return retcode;
485 }
There's bug here.
So, how about to wrap operator= invocation in atp_struct::setDiagsArea(), and in operator= we deal with the ref count, like Qifan suggested?
-----邮件原件-----
发件人: Selva Govindarajan <se...@esgyn.com>
发送时间: 2018年8月15日 0:23
收件人: dev@trafodion.apache.org
主题: RE: refCount on ComDiagsArea
I second Dave's comment. I was about to comment in similar lines. When I saw Dave's message, I had no second thoughts to second it.
Selva
-----Original Message-----
From: Qifan Chen <qi...@esgyn.com>
Sent: Tuesday, August 14, 2018 9:11 AM
To: dev@trafodion.apache.org
Subject: Re: refCount on ComDiagsArea
Hi,
I personally also like the 2nd option which is to overload the operator= for ComDiagsArea.
But because of high number of calls to atp_struct::setDiagsArea() in the executor, it is better to hide the call to operator=(ComDiagsArea&) inside atp_struct::setDiagsArea().
Also keep in mind that we can not ubiquitously use C11 structs such as shared_ptr or make_shared in our code base yet since we still need to support platforms such as CentOS6 that do not have C11 support package.
Thanks --Qifan
________________________________
From: Dave Birdsall <da...@esgyn.com>
Sent: Tuesday, August 14, 2018 10:10:43 AM
To: dev@trafodion.apache.org
Subject: RE: refCount on ComDiagsArea
Hi,
I have not researched this area, but it strikes me as one that could be very delicate. It may be that in most code paths it is assumed that some other function is incrementing the reference count. Great care should be taken in modifying this otherwise it may lead to memory leaks. I am hoping others who are more knowledgeable will add to this discussion.
Can you give more insight into what problem led you here?
Dave
-----Original Message-----
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 4:11 AM
To: dev@trafodion.apache.org
Subject: refCount on ComDiagsArea
hi,
When setting a ComDiagsArea, I find the refCount did not increase, in function atp_struct::setDiagsArea of file core/sql/exp/ExpAtp.h:
inline void atp_struct::setDiagsArea(ComDiagsArea* diagsArea) {
if (diagsArea_)
diagsArea_->decrRefCount();
diagsArea_ = diagsArea;
}
I guess this is a problem.
There are two solutions to fix this:
1. Invoke incrRefCount for ComDiagsArea to increase, just after the assignment.
2. Overload the operator= for ComDiagsArea, to increase within the operator=,
I find operator= is declared in ComDiags.h, but there is no implementation.
The 2nd solution may be better, as the both increment for the left-hand side ComDiagsArea and the decrement for the right-hand side ComDiagsAre can be handled within a single operator=, which is friendly to users, like shared_ptr in C++.
Regards,
Wenjun Zhu
RE: refCount on ComDiagsArea
Posted by Selva Govindarajan <se...@esgyn.com>.
I second Dave's comment. I was about to comment in similar lines. When I saw Dave's message, I had no second thoughts to second it.
Selva
-----Original Message-----
From: Qifan Chen <qi...@esgyn.com>
Sent: Tuesday, August 14, 2018 9:11 AM
To: dev@trafodion.apache.org
Subject: Re: refCount on ComDiagsArea
Hi,
I personally also like the 2nd option which is to overload the operator= for ComDiagsArea.
But because of high number of calls to atp_struct::setDiagsArea() in the executor, it is better to hide the call to operator=(ComDiagsArea&) inside atp_struct::setDiagsArea().
Also keep in mind that we can not ubiquitously use C11 structs such as shared_ptr or make_shared in our code base yet since we still need to support platforms such as CentOS6 that do not have C11 support package.
Thanks --Qifan
________________________________
From: Dave Birdsall <da...@esgyn.com>
Sent: Tuesday, August 14, 2018 10:10:43 AM
To: dev@trafodion.apache.org
Subject: RE: refCount on ComDiagsArea
Hi,
I have not researched this area, but it strikes me as one that could be very delicate. It may be that in most code paths it is assumed that some other function is incrementing the reference count. Great care should be taken in modifying this otherwise it may lead to memory leaks. I am hoping others who are more knowledgeable will add to this discussion.
Can you give more insight into what problem led you here?
Dave
-----Original Message-----
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 4:11 AM
To: dev@trafodion.apache.org
Subject: refCount on ComDiagsArea
hi,
When setting a ComDiagsArea, I find the refCount did not increase, in function atp_struct::setDiagsArea of file core/sql/exp/ExpAtp.h:
inline void atp_struct::setDiagsArea(ComDiagsArea* diagsArea) {
if (diagsArea_)
diagsArea_->decrRefCount();
diagsArea_ = diagsArea;
}
I guess this is a problem.
There are two solutions to fix this:
1. Invoke incrRefCount for ComDiagsArea to increase, just after the assignment.
2. Overload the operator= for ComDiagsArea, to increase within the operator=,
I find operator= is declared in ComDiags.h, but there is no implementation.
The 2nd solution may be better, as the both increment for the left-hand side ComDiagsArea and the decrement for the right-hand side ComDiagsAre can be handled within a single operator=, which is friendly to users, like shared_ptr in C++.
Regards,
Wenjun Zhu
Re: refCount on ComDiagsArea
Posted by Qifan Chen <qi...@esgyn.com>.
Hi,
I personally also like the 2nd option which is to overload the operator= for ComDiagsArea.
But because of high number of calls to atp_struct::setDiagsArea() in the executor, it is better to hide the call to operator=(ComDiagsArea&) inside atp_struct::setDiagsArea().
Also keep in mind that we can not ubiquitously use C11 structs such as shared_ptr or make_shared in our code base yet since we still need to support platforms such as CentOS6 that do not have C11 support package.
Thanks --Qifan
________________________________
From: Dave Birdsall <da...@esgyn.com>
Sent: Tuesday, August 14, 2018 10:10:43 AM
To: dev@trafodion.apache.org
Subject: RE: refCount on ComDiagsArea
Hi,
I have not researched this area, but it strikes me as one that could be very delicate. It may be that in most code paths it is assumed that some other function is incrementing the reference count. Great care should be taken in modifying this otherwise it may lead to memory leaks. I am hoping others who are more knowledgeable will add to this discussion.
Can you give more insight into what problem led you here?
Dave
-----Original Message-----
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 4:11 AM
To: dev@trafodion.apache.org
Subject: refCount on ComDiagsArea
hi,
When setting a ComDiagsArea, I find the refCount did not increase, in function atp_struct::setDiagsArea of file core/sql/exp/ExpAtp.h:
inline void atp_struct::setDiagsArea(ComDiagsArea* diagsArea) {
if (diagsArea_)
diagsArea_->decrRefCount();
diagsArea_ = diagsArea;
}
I guess this is a problem.
There are two solutions to fix this:
1. Invoke incrRefCount for ComDiagsArea to increase, just after the assignment.
2. Overload the operator= for ComDiagsArea, to increase within the operator=,
I find operator= is declared in ComDiags.h, but there is no implementation.
The 2nd solution may be better, as the both increment for the left-hand side ComDiagsArea and the decrement for the right-hand side ComDiagsAre can be handled within a single operator=, which is friendly to users, like shared_ptr in C++.
Regards,
Wenjun Zhu
RE: refCount on ComDiagsArea
Posted by Dave Birdsall <da...@esgyn.com>.
Hi,
I have not researched this area, but it strikes me as one that could be very delicate. It may be that in most code paths it is assumed that some other function is incrementing the reference count. Great care should be taken in modifying this otherwise it may lead to memory leaks. I am hoping others who are more knowledgeable will add to this discussion.
Can you give more insight into what problem led you here?
Dave
-----Original Message-----
From: Zhu, Wen-Jun <we...@esgyn.cn>
Sent: Tuesday, August 14, 2018 4:11 AM
To: dev@trafodion.apache.org
Subject: refCount on ComDiagsArea
hi,
When setting a ComDiagsArea, I find the refCount did not increase, in function atp_struct::setDiagsArea of file core/sql/exp/ExpAtp.h:
inline void atp_struct::setDiagsArea(ComDiagsArea* diagsArea) {
if (diagsArea_)
diagsArea_->decrRefCount();
diagsArea_ = diagsArea;
}
I guess this is a problem.
There are two solutions to fix this:
1. Invoke incrRefCount for ComDiagsArea to increase, just after the assignment.
2. Overload the operator= for ComDiagsArea, to increase within the operator=,
I find operator= is declared in ComDiags.h, but there is no implementation.
The 2nd solution may be better, as the both increment for the left-hand side ComDiagsArea and the decrement for the right-hand side ComDiagsAre can be handled within a single operator=, which is friendly to users, like shared_ptr in C++.
Regards,
Wenjun Zhu