ATLASRECTS-4956 Migrate SharedObject to be a type alias to std::shared_ptr
SharedObject
--> std::shared_ptr
The SharedObject among other things had an unsafe reference count.
In reality looks like a kind C++98 implementation of shared (ref counted) pointer.
This MR makes SharedObject to be an alias for shared_ptr.
Trivial changes for clients
Most of the changes so far on the clients are
- Mainly
foo->getPtr()
-->foo->get()
(seem like ~100) - A few
return foo->getRef()
-->return *(foo->get())
No Delete Option of SharedObject (the tricky bit)
The SharedObject class had a "not delete" option
// given bool = true means object gets never deleted */
SharedObject(T* tobj, bool ndel=false) :
The option ndel
was set to true
in 6-7 places.
It signifies that you have a ptr that will be deleted from somewhere else,
but still you want to construct a shared object with it . The reason are beyond this MR.
Obviously there is not std::shared-ptr(T* ptr, bool)
.
there is
shared_ptr( std::nullptr_t ptr, Deleter d )
So in this case the ndel=true
has been replaced
with a shared_ptr with no-op deleter ( a nodelete deleter if it makes any sense)
to try and retain the no delete semantics...
e.g
Trk::SharedObject<const Trk::TrackingVolume>(sVol, true)
which was not supposed to delete its element (due to the ndel=true) looks like
Trk::SharedObject<const Trk::TrackingVolume>(sVol, [](const Trk::TrackingVolume*){})
while
Trk::SharedObject<const Trk::TrackingVolume>(sVol,false)
which was the default SharedObject behaviour trivially becomes:
Trk::SharedObject<const Trk::TrackingVolume>(sVol)
One could imagine in the future that one would
like some people closer to each code to take
a critical look at the ownership relations that required nodel=true
.....
Further clarification on the ndel replacement
For starters it feels like replacing a hack with a hack. So perhaps we should really understand what we want to do here long term look last phrase above.
Now, for some reason let's assume you have a ptr,
and you want to pass it to a shared_ptr
while retaining ownership (for some reason) of when it will be deleted.
Similar to the ndel
option of the SharedObject.
A simple example (to try at home) would be
#include <memory>
int main(){
double* a = new double(10.90);
//Now we want to both pass it in shared_ptr
//and retain ownership. For some reason we
// want this... i.e ndel
//way to achieve it
std::shared_ptr<double> aShared=std::shared_ptr<double>(a,[](double*){});
//segmantation from double delete
//std::shared_ptr<double> aShared=std::shared_ptr<double>(a);
delete a;
return 0;
}
Wrapping option
Another option would have been to wrap a shared_ptr inside this class. In principle an alias should make migrating and communicating with shared_ptr easier for the future, so was deemed to be preferred in the JIRA discussion.
Such a direct wrapper needing no client modification, would naively look like this.
class SharedObject{
public:
SharedObject():m_ptr{}{
}
SharedObject(T* ptr, bool nodel=false){
if(nodel==false){
m_ptr=std::shared_ptr<T>(ptr);
}else{
m_ptr=std::shared_ptr<T>(ptr,[](T*){});
}
}
SharedObject(const SharedObject& so): m_ptr{so.m_ptr}{
}
SharedObject& operator=(const SharedObject& so){
if(this!=&so){
this->m_ptr=so.m_ptr;
}
return *this;
}
T& operator*() const { return (*m_ptr);}
T* operator->() const { return m_ptr; }
T& getRef() const { return (*m_ptr); }
T* getPtr() const { return m_ptr; }
private:
std::shared_ptr<T> m_ptr;
};
The replacement in the client code maps closely to it. As in some sense the code had to be put into place at the client side than be "wrapped"
Mentioning @ssnyder