CODEWITHSUNDEEP
c++arraysthread-safetydestructorsmart-pointers
tge
tge

Reputation: 91

C++ destructor called on array index? Crash on non-thread-safe ref-counting objects

The following code (from Apache Tuscany SDO C++) occasionally (actually very rarely) causes subsequent crashes and I don't understand what's going on. The following statement is in DataObjectImpl.cpp (see stack below):

PropertyImpl* DataObjectImpl::getPropertyImpl(unsigned int index)
{
...
904 PropertyList props = getType().getProperties();
905 if (index < props.size())
906 {
907   return (PropertyImpl*)&props[index];
...

causes the following stack (all omitted frames above and below look plausible):

Note: #11 libtuscany_sdo.dll!std::vector<>::~vector<>                                [c:\program files\microsoft visual studio 9.0\vc\include\vector:559]
Note: #12 libtuscany_sdo.dll!commonj::sdo::PropertyList::~PropertyList               [y:\external\tuscany\src\runtime\core\src\commonj\sdo\propertylist.cpp:60]
Note: #13 libtuscany_sdo.dll!commonj::sdo::DataObjectImpl::getPropertyImpl           [y:\external\tuscany\src\runtime\core\src\commonj\sdo\dataobjectimpl.cpp:907]
Note: #14 libtuscany_sdo.dll!commonj::sdo::DataObjectImpl::getSDOValue               [y:\external\tuscany\src\runtime\core\src\commonj\sdo\dataobjectimpl.cpp:3845]

The actual question is - why is the destructor of PropertyList called??

As stated, the stack looks OK otherwise, also the vector destructor, as PropertyList has a member std::vector<PropertyImplPtr> plist; and the array index operator of PropertyList just calls the array index of the plist member.

And, even more puzzling (to me), why this happens only occasionally ... Many thx!!

EDIT: Unfortunately my original question was wrong (my misinterpretation): yes, the call of the destructor is OK, as answered/commented/explained. I investigated the problem further and am pretty certain to understand what the real problem is - see own answer below, possibly it may help other people ... To cover both topics I slightly changed the title and extended the list of tags. Hope that's OK ...

Upvotes: 0

Views: 184

Answers (3)

tge
tge

Reputation: 91

My original question was mis-stated, I was thinking the deletion of PropertyList causes the crash of my prog. Effectively this is the case, but the actual reason is, as I believe, here (just in case, somebody cares):

My prog runs multiple threads doing the same thing. It's working with SDO objects a lot, which in turn use a RefCountingPtr mechanism at many places. Normally this works nicely, but as stated originally, occasionally my prog crashes. A concept of C++ SDO is that it is type-based, types are defined using XSD, at the beginning my prog loads the XSD into a global type repo shared among all threads (because I was thinking, this happens once, after that the types are read-only, so it's safe doing this). A type has a list of properties and each property is represented by a PropertyImplPtr, which in turn is such a RefCountingPtr. Looks like this:

class TypeImpl {
...
std::vector<PropertyImplPtr> props;
...

typedef RefCountingPointer<PropertyImpl> PropertyImplPtr;

Now, when the PropertyList gets deleted as in the original post, the contained vector of PropertyImplPtr gets freed, hence the reference count decreases and if zero, the object deletes itself. Exactly this happens in my crash, only that I omitted that part of the stack because I didn't think it is important:

Note: # 0 replace_operator_delete                                                    [d:\drmemory_package\common\alloc_replace.c:2524]
Note: # 1 libtuscany_sdo.dll!commonj::sdo::PropertyImpl::`vector deleting destructor'
Note: # 2 libtuscany_sdo.dll!commonj::sdo::RefCountingObject::releaseRef             [y:\external\tuscany\src\runtime\core\src\commonj\sdo\refcountingobject.cpp:71]
Note: # 3 libtuscany_sdo.dll!commonj::sdo::RefCountingPointer<>::~RefCountingPointer<> [y:\external\tuscany\src\runtime\core\src\commonj\sdo\refcountingpointer.h:146]
Note: # 4 libtuscany_sdo.dll!commonj::sdo::RefCountingPointer<>::`scalar deleting destructor'

So, the question now is how it is possible that the refcount is zero? Looks as if the RefCounting implementation is not thread safe, e.g.:

template<class T>
void RefCountingPointer<T>::init()
{
    if (pointee == 0) return;
    pointee->addRef();
}
...
template<class T>
/*SDO_API*/ RefCountingPointer<T>& RefCountingPointer<T>::operator=(const RefCountingPointer& rhs)
{
    if (pointee != rhs.pointee)
    {
        T *oldP = pointee;
        pointee = rhs.pointee;
        init();
        if (oldP) oldP->releaseRef();
    }
    return *this;
}

Here, pointee gets an additional reference, while the refcount will be incremented in init(). If now another thread wants to delete such an object, it may do that because the refcount is still 1.

Upvotes: 0

AnT stands with Russia
AnT stands with Russia

Reputation: 320671

PropertyList props is a local variable inside getPropertyImpl function. Once getPropertyImpl completes, props gets automatically destroyed and the destructor of PropertyList is called. Apparently this is the destructor call that you see.

If getType().getProperties() returns its result by reference, then you are probably supposed to do

PropertyList &props = getType().getProperties();

But if it returns PropertyList by value, then there's no way you can do 

return (PropertyImpl*)&props[index];

with props declared as a local variable. The local variable gets killed at the function exit, the returned pointer becomes invalid (with the symptoms you describe: sometimes it "works", sometimes it doesn't).

BTW, why are you casting the result of &props[index]? What is the original type of &props[index]?

Upvotes: 1

Dimitrios Bouzas
Dimitrios Bouzas

Reputation: 42929

  • Variable PropertyList props has local scope (i.e., the scope of member function DataObjectImpl::getPropertyImpl).

  • After the return statement in member function DataObjectImpl::getPropertyImpl the destructor of variable PropertyList props is evoked and thus the object props is destroyed.

  • Thus, you are returning the address of a destroyed object. Accessing an already destroyed object causes undefined behaviour.

Solution:

Return a clone of the object you want to return:

std::unique_ptr<PropertyImpl> 
DataObjectImpl::getPropertyImpl(unsigned int index) {
  PropertyList props = getType().getProperties();
  if (index < props.size()) {
    return std::unique_ptr<PropertyImpl>(new PropertyImpl(props[index]));
  ...

Upvotes: 2

Related Questions