SmartPointer (aka Handle) question

g00fy · June 2006

Hi,

I wanted to know if it's legal to pass 'this' to a constructor/function which expects a Handle?

How will it determine it's referencecount? Because to me it looks like:
- pass *this*
- incRef [ ref = 1 ]
- ... do something
- decRef @ destructor of Handle [ ref = 0 ]
- --> destroy *this*

Which ofcourse isn't good as I just passed *this* expecting it would survive the Handle.

What I also experienced is that when I'm working in a function called remotely (proxy->function), if I pass *this*, where *this* is the proxy I'm working on... It's not a problem at all... It increases nicely (the reference count) and decreases nicely, but never gets to 0, so it's not destroyed...

But if I just do it completely locally in a Handle, it will increase to 1, and decrease to 0, destroying the *this* object...

Could you give me any pointers on how to interpret this powerfull feature of SmartPointers in ICE?

Thanks a lot!

marc · June 2006

You can temporarily disable destruction of the pointee when the reference count hits zero. We use this in a few places in our own code. For example:

Foo::Foo()
{
    __setNoDelete(true);
    try
    {
	some_function(this);
    }
    catch(...)
    {
	__setNoDelete(false);
	throw;
    }
    __setNoDelete(false);
}

void some_function(const Handle<Foo>& foo)
{
}

Without the calls to __setNoDelete(), the reference count would become zero after the call to some_function(), and the Foo object would destroy itself.

g00fy · June 2006

Hmmm thanks Marc.

But what if you have a class like:

class Cat : public Animal
{
public:
  Cat( const Handle<House>& house )
  : m_house(house)
  {
  }

private:
  Handle<House> m_house;
}

...

Cat * cat = new Cat(this);

Where 'this' points to a class 'House' for example.

Where would you put the __setNoDelete functions then? Because I guess from the code you've written it's some kind of global don't delete function...

The Handles, do they work based on the pointer? For example an internal mapping from pointer -> reference count, or is it stored in the object itselves GCShared?)?

Maybe the previous example is just a bad design, it's possible ofcourse...

Maybe a little more indepth example picturing the structure of my project:

- I have a Factory
- It create()'s Car*'s
- Inside the Car you can have - for example - seats: CreateSeats( color, ..., CarPtr );

Now there the problem arises... Because I want to be able to return from a seat to it's car, i need some way to do it... I did it by having a function in the Factory keeping track of every car made, and whenever a SeatI is constructed, I call the Factory to ask... Please give me the Car with this identification number. Then the Factory will provide me with a CarPtr, which I pass through the Seat-constructor.
I can feel it in my guts this is absolutely wrong design, but I really have no idea how to implement the Car* getParent() function otherwise...

That's where I used to use 'this' instead of the call to the Factory... And that's where it used to crash untill I figured out it was because the reference count dropped 0 sometimes... Because if you destroy a Seat, the reference count for the "Handle<Car> _parent" becomes 0, destroying the object Car, which ofcourse is not correct as it might have still other things in there...

Thanks if you could clarify me what I should use to counter this kind of 'parent'-question.

marc · June 2006

g00fy wrote:
Hmmm thanks Marc.

But what if you have a class like:
class Cat : public Animal
{
public:
  Cat( const Handle<House>& house )
  : m_house(house)
  {
  }

private:
  Handle<House> m_house;
}

...

Cat * cat = new Cat(this);
Where 'this' points to a class 'House' for example.

Where would you put the __setNoDelete functions then? Because I guess from the code you've written it's some kind of global don't delete function...

I'm afraid I don't understand the question. Why would you need the __setNoDelete function in the code above?

__setNoDelete is not a global function, it's a member function of the base class for reference counted objects.

g00fy wrote:

The Handles, do they work based on the pointer? For example an internal mapping from pointer -> reference count, or is it stored in the object itselves GCShared?)?

It is stored in GCShared. This is called "instrusive reference counting".

g00fy wrote:

Maybe the previous example is just a bad design, it's possible ofcourse...

Maybe a little more indepth example picturing the structure of my project:

- I have a Factory
- It create()'s Car*'s
- Inside the Car you can have - for example - seats: CreateSeats( color, ..., CarPtr );

Now there the problem arises... Because I want to be able to return from a seat to it's car, i need some way to do it... I did it by having a function in the Factory keeping track of every car made, and whenever a SeatI is constructed, I call the Factory to ask... Please give me the Car with this identification number. Then the Factory will provide me with a CarPtr, which I pass through the Seat-constructor.
I can feel it in my guts this is absolutely wrong design, but I really have no idea how to implement the Car* getParent() function otherwise...

That's where I used to use 'this' instead of the call to the Factory... And that's where it used to crash untill I figured out it was because the reference count dropped 0 sometimes... Because if you destroy a Seat, the reference count for the "Handle<Car> _parent" becomes 0, destroying the object Car, which ofcourse is not correct as it might have still other things in there...

Thanks if you could clarify me what I should use to counter this kind of 'parent'-question.

Let me suggest that you provide a concrete code example instead of explaining the problem, then I tell you how to solve the problem

g00fy · June 2006

Hey, hereby a code example:

Slice definitions (highly simplified):

/** Factory.ice **/
  interface Factory
  {
    Car* create( string type );
  }

/** Car.ice **/
  interface Car
  {
    Seat* createSeats( );
  }

/** Seat.ice **/
  interface Seat
  {
    Car* getParent( );
  }

Implementation:

/* FactoryI.h */
class FactoryI : public virtual Factory
{
public:
  virtual CarPrx create( const string& type, const Ice::Current& = Ice::Current() );

  CarPrx getMyPrx( const string& ID ) const;

private:
  map<string, CarPrx> carTypes;
};

/* FactoryI.cpp */
CarPrx
FactoryI::create( const string& type, const Ice::Current& c )
{
  // do some checks
  ...
  // create the actual object
  CarIPtr car = new CarI( *this, type );
  const string id = IceUtil::generateUUID();
  car->SetID( id );
  CarPrx prx = CarPrx::unchecked_Cast(c.adapter->addWithUUID(car));
  carTypes[ id ] = prx;

  return prx;
}

CarPrx
Factory::getMyPrx( const string& ID ) const
{
  map<string, CarPrx>::const_iterator ci;
  if ( (ci = carTypes.find(ID)) == carTypes.end() )
  {
    return 0;
  }
  else
  {
    return ci->second;
  }
}

/* Car.h */
class CarI : public virtual Car
{
public:
  CarI( Factory& fac, const string& type );

  virtual SeatPrx createSeats( const Ice::Current& c = Ice::Current() );

  void SetID( const string& serial );

private:
  string m_type, m_ID; 
  Factory& parent;
}

/* Car.cpp */
CarI::CarI( Factory& fac, const string& type )
: m_type(type),
  parent(fac)
{
}

void
CarI::SetID( const string& serial )
{
  m_ID = serial;
}

SeatPrx
CarI::createSeats( const Ice::Current& c )
{
  SeatPtr seat = new SeatI( parent.GetMyPrx(m_ID) );
  return SeatPrx::uncheckedCast( c.adapter->addWithUUID(seat) );
}

/* Seat.h */
class SeatI : public virtual Seat
{
public:
  SeatI( const CarPrx& parent );

  virtual CarPrx getParent() { return m_parent; }

private:
  CarPrx m_parent;
};

/* Seat.cpp */
SeatI::SeatI( const CarPrx& parent )
: m_parent(parent)
{
}

That's basically it... And this is what bothers me (although it works fine):
--
SeatPtr seat = new SeatI( parent.GetMyPrx(m_ID) );
--

Hope you know a better way of doing it

Greetz,

marc · June 2006

Don't use Factory& as data member! Use FactoryPtr, otherwise you bypass reference counting.

g00fy · June 2006

Well, there can be only 1 Factory... You can consider it a global object... It's constructed inside the main(argc, argv) function, to be never touched again... Unless you stop the program, but then you cancel all the Car's anyone.

However this is of very little concern to me

The biggest concern is how to *properly* implement the getParent() function

?

Thanks,

marc · June 2006

g00fy wrote:

Well, there can be only 1 Factory... You can consider it a global object... It's constructed inside the main(argc, argv) function, to be never touched again... Unless you stop the program, but then you cancel all the Car's anyone.

Global or not, you must not create servants or other reference-counted objects on the stack, and you should always refer to them using a handle, not a C++ reference. Otherwise it's easy to screw up the reference count.

g00fy wrote:

However this is of very little concern to me The biggest concern is how to *properly* implement the getParent() function ?

I still don't understand... what exactly is not properly done in this function?

g00fy · June 2006

Hmmm ok,

But if I change my source like:

CarI::CarI( const FactoryPtr& fac )
: parent(fac)
{
}

And then I pass '*this' to the constructor... Won't the FactoryPtr reference count be increased to 1 upon assignment inside the CarI-constructor? And won't it be dropped to 0 upon destruction? But I guess that I should be useing something like the __setNoDelete(true) then.

Why I say not properly done is because it looks like not very "neat" to me... Like maybe you know some easier way to do it then to keep a mapping of some ID and related proxies inside the main Factory. I was more thinking of something like some kind of internal function or some way to pass *this* to the SeatI-constructor without actually destroying the CarI upon destroying the SeatI.

Could you tell me if this would have that effect:

CarIPtr car( this );
cat->__setNoDelete(true);
SeatI seat = new SeatI(car);

Because if I can do this way, I create a new CarPtr, with *this*, which won't get destructed upon destroying SeatI.

I also saw you were setting it back to false, but if I never want it to be destroyed (*this* shouldn't be destroyed by the Handle), then I can just leave it like that right?

marc · June 2006

g00fy wrote:
Hmmm ok,

But if I change my source like:
CarI::CarI( const FactoryPtr& fac )
: parent(fac)
{
}
And then I pass '*this' to the constructor... Won't the FactoryPtr reference count be increased to 1 upon assignment inside the CarI-constructor? And won't it be dropped to 0 upon destruction? But I guess that I should be useing something like the __setNoDelete(true) then.

Why? You store the parent in a handle, which increases the count. Nothing will get deleted. No calls to __setNoDelete() are necessary.

g00fy wrote:

Why I say not properly done is because it looks like not very "neat" to me... Like maybe you know some easier way to do it then to keep a mapping of some ID and related proxies inside the main Factory. I was more thinking of something like some kind of internal function or some way to pass *this* to the SeatI-constructor without actually destroying the CarI upon destroying the SeatI.

Can you show me the concrete code that passes "this" and causes the destruction?

g00fy wrote:
Could you tell me if this would have that effect:
CarIPtr car( this );
cat->__setNoDelete(true);
SeatI seat = new SeatI(car);
Because if I can do this way, I create a new CarPtr, with *this*, which won't get destructed upon destroying SeatI.

I also saw you were setting it back to false, but if I never want it to be destroyed (*this* shouldn't be destroyed by the Handle), then I can just leave it like that right?

Sorry, I'm afraid I still can't follow you. Please show me a simple code example that causes a crash if "this" is passed. Make the example self-contained, don't allocate servants on the stack, use handles and no C++ references, and mark exactly the place where it crashes. Otherwise it is very difficult for me to help you.

g00fy · June 2006

Hi Marc,

I once made a test program which crashed upon such thing as we're talking about here now... I still had that in my mind.
So I was trying now to recreate it, but seems to miserably fail to do it.

Probably it was a mix of C-pointer, C-references and Ice-handlers... Right now I did this:

#include <IceUtil/Handle.h>
#include <IceUtil/Shared.h>
#include <iostream>


class A;
typedef IceUtil::Handle<A> APtr;
void Try( APtr p );


class A /*: public IceUtil::Shared*/
{
public:
  A( int i = 0 )
  : m_i(i)
  {

  }

  int getI() const { return m_i; }

  void Try( ) { ::Try(this); }

private:
  int m_i;
};


void Try( APtr p )
{
  std::cout << p->getI() << std::endl;
}

int _tmain(int argc, _TCHAR* argv[])
{
  APtr a = new A(1);
  a->Try();
  a->Try();
  a->Try();

  return 0;
}

Having the "public IceUtil::Shared" commented, it will give an error of not finding __*ref functions.
So I really don't remember how I did it, but seems like if I make every object via slice (so basically going back to Ice::Object & Ice::GCShared), it will have a reference counting, and passing a *this* won't cause ANY problems at all

Which answered my question, and might shed some light on why I was still pursuing the wrong idea that passing a *this* could cause improper pointer deletion.

Thanks for your patience Marc!

SmartPointer (aka Handle) question

Comments