0

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 ...

tge
  • 91
  • 9
  • 6
    I'm 99% sure it's http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope – chris Aug 21 '14 at 22:09
  • Because you are `return`ing and so `props`, a local variable, must be destroyed? – T.C. Aug 21 '14 at 22:09

3 Answers3

2

  • 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]));
  ...
101010
  • 41,839
  • 11
  • 94
  • 168
  • It may be better to simply reference the property list. – Puppy Aug 21 '14 at 22:22
  • @Puppy I though of this solution at first. However, I don't know what `getType().getProperties()` returns and what's its scope. – 101010 Aug 21 '14 at 22:24
  • Hmm, I'm afraid that you are right and that this is most probably (this is actually not my code) even intended. This again means that my actual problem is elsewhere ... well, many thx anyway!! – tge Aug 21 '14 at 22:25
  • The return statement is actually OK, I suppose, because that code is not supposed to return the actual prop list but rather one array element only, so destroying the actual propList and the embedded vector should not damage anything. Will have to dig further ... – tge Aug 21 '14 at 22:31
1

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]?

AnT stands with Russia
  • 312,472
  • 42
  • 525
  • 765
  • About the casting: looks as if they (again, not my code) want to hide the internal PropertyImpl and expose just Property. To my understanding that return statement first extracts one pointer from the array, then returns that pointer and upon returning the array gets destroyed - probably OK. So, I actually asked the wrong question ... understanding this is already some help :-) – tge Aug 21 '14 at 23:02
  • @tge: That still does not explain the need for this cast. The return argument is casting to `PropertyImpl *` and the whole function returns `PropertyImpl *`. How does that help to "hide" `PropertyImpl`? – AnT stands with Russia Aug 21 '14 at 23:12
  • I guess, because its an API and the API users are supposed to use Property which is visible in the headers, while in the C++ sources they use PropertyImpl. But possibly the cast is just unneeded. If you really want to know, look for "Apache Tuscany SDO C++" or so, its open source. But many thx in any case!! – tge Aug 22 '14 at 08:30
0

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.

tge
  • 91
  • 9