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 <[email protected]> Sent: Tuesday, August 14, 2018 9:11 AM To: [email protected] 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 <[email protected]> Sent: Tuesday, August 14, 2018 10:10:43 AM To: [email protected] 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 <[email protected]> Sent: Tuesday, August 14, 2018 4:11 AM To: [email protected] 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
