What kind of pattern is this?

[Marcel Müller]

I have a class that destroys itself after it has done its job. Something
like a thread, but without a thread.

// Some Object
class MyObject;

// Schedule a task for obj in the Worker Queue.
void ScheduleObject(MyObject* obj);

class RescheduleWorker
{private:
intrusive_ptr<MyObject> Object;
vector<intrusive_ptr<MyObject> > Dependencies;
public:
RescheduleWorker(MyObject& obj,
const vector<intrusive_ptr<MyObject> >& dependencies)
: Object(&obj), Dependencies(dependencies)
{ // register some callbacks in the dependency list
}
private:
~RescheduleWorker() {}
// Called by some internal observer functions exactly once
// when a complex dependency condition is true.
void OnCompleted()
{ ScheduleObject(Object.get());
delete this;
}
};


// usage:
new RescheduleWorker(obj, dependencies);

The latter looks a bit strange, because the pointer returned by new is
immediately discarded. Is this a common pattern? How is it called?


Marcel

 

[Victor Bazarov]

I have a class that destroys itself after it has done its job. Something
like a thread, but without a thread.

// Some Object
class MyObject;

// Schedule a task for obj in the Worker Queue.
void ScheduleObject(MyObject* obj);

class RescheduleWorker
{private:
intrusive_ptr<MyObject> Object;
vector<intrusive_ptr<MyObject> > Dependencies;
public:
RescheduleWorker(MyObject& obj,
const vector<intrusive_ptr<MyObject> >& dependencies)
: Object(&obj), Dependencies(dependencies)
{ // register some callbacks in the dependency list
}
private:
~RescheduleWorker() {}
// Called by some internal observer functions exactly once
// when a complex dependency condition is true.
void OnCompleted()
{ ScheduleObject(Object.get());
delete this;
}
};


// usage:
new RescheduleWorker(obj, dependencies);

The latter looks a bit strange, because the pointer returned by new is
immediately discarded. Is this a common pattern? How is it called?

Your 'OnCompleted' function is never called from anywhere... It's
called "dead code".

V

 

[Richard Damon]

Your 'OnCompleted' function is never called from anywhere... It's called
"dead code".

My guess is that 'OnCompleted' is called as part of the call back
architecture mentioned in the constructor (but omitted from the example).

 

[Werner]

I have a class that destroys itself after it has done its job. Something
like a thread, but without a thread.

// Some Object
class MyObject;

// Schedule a task for obj in the Worker Queue.
void ScheduleObject(MyObject* obj);

class RescheduleWorker
{private:
   intrusive_ptr<MyObject> Object;
   vector<intrusive_ptr<MyObject> > Dependencies;
  public:
   RescheduleWorker(MyObject& obj,
     const vector<intrusive_ptr<MyObject> >& dependencies)
   : Object(&obj), Dependencies(dependencies)
   { // register some callbacks in the dependency list
   }
  private:
   ~RescheduleWorker() {}
   // Called by some internal observer functions exactly once
   // when a complex dependency condition is true.
   void OnCompleted()
   { ScheduleObject(Object.get());
     delete this;
   }

};

// usage:
new RescheduleWorker(obj, dependencies);

The latter looks a bit strange, because the pointer returned by new is
immediately discarded. Is this a common pattern? How is it called?

Marcel

The object would be delete when OnCompleted is called. It
would not be necessary to keep the pointer. IMHO it would
have been better to (in this case) have a public function ...

"static void RescheduleWorker::spawn(
MyObject&,
const vector<intrusive_ptr<MyObject> >& )"

.... especially seeing that the pointer is never used,
and make the constructor private.

Regards,

Werner

 

[Krice]

I have a class that destroys itself after it has done its job.

Check out "local instance". It does what you try to do in proper way.

 

[Werner]

Check out "local instance". It does what you try to do in proper way.

Nope. I can't seem to find "Local Instance" between all the clutter.
Care to give a link?

Kind Regards,

Werner

 

[Goran]

I have a class that destroys itself after it has done its job. Something
like a thread, but without a thread.

// Some Object
class MyObject;

// Schedule a task for obj in the Worker Queue.
void ScheduleObject(MyObject* obj);

class RescheduleWorker
{private:
   intrusive_ptr<MyObject> Object;
   vector<intrusive_ptr<MyObject> > Dependencies;
  public:
   RescheduleWorker(MyObject& obj,
     const vector<intrusive_ptr<MyObject> >& dependencies)
   : Object(&obj), Dependencies(dependencies)
   { // register some callbacks in the dependency list
   }
  private:
   ~RescheduleWorker() {}
   // Called by some internal observer functions exactly once
   // when a complex dependency condition is true.
   void OnCompleted()
   { ScheduleObject(Object.get());
     delete this;
   }

};

// usage:
new RescheduleWorker(obj, dependencies);

The latter looks a bit strange, because the pointer returned by new is
immediately discarded. Is this a common pattern? How is it called?

"delete this" is seen in places, and, beside intrusive refcounting, I
am yet to see a reason why it is a good idea. In your case, ownership
of the object is effectively taken by whoever is going to call
OnCompleted. So that somebody can do

p->OnCompleted();
delete p;

or better yet

auto_ptr<ReshceduleWorker>(p)->OnCompleted();

and "delete this" should disappear.

"delete this" is usually a sign of underdesigned heap object
ownership.

Goran.

 

[Juha Nieminen]

"delete this" is seen in places, and, beside intrusive refcounting, I
am yet to see a reason why it is a good idea.

It's still unclear to me if 'delete this' is defined or undefined
behavior according to the standard. I wouldn't be surprised if it was
undefined because I'm assuming the standard does not want to forbid
a compiler from putting some kind of "cleaning code" at the end of a
member function if it needs to do that for whatever reason. If this
"cleaning code" accesses the object pointed by 'this', we have a problem.

(Tangentially, if in Objective C you do a "[self release]", which is
equivalent to "delete this" in C++, there's a big chance the program will
crash if the retain count was 1 before that line. Incidentally, there's
a trick that can be used to circumvent this, and it's to perfom a
"[self autorelease]", which will delay the destruction to when the
execution returns to the main runloop, at which point it's safe to
destroy the object as it's not being used by anything anymore. Of course
there's no such a thing in C++, unless you implement this kind of
functionality explicitly.)

 

[Marcel Müller]

The object would be delete when OnCompleted is called. It
would not be necessary to keep the pointer. IMHO it would
have been better to (in this case) have a public function ...

"static void RescheduleWorker::spawn(
MyObject&,
const vector<intrusive_ptr<MyObject> >& )"

... especially seeing that the pointer is never used,
and make the constructor private.

A good point. It makes things more clear. Especially it prevents from
writing RescheduleWorker(obj, dependencies) (without new) accidentally.


Marcel

 

[Juha Nieminen]

'delete this' is not undefined behaviour; 'delete this' is perfectly fine.

It's not that I doubt you, but it would be nice to see the part of the
standard that specifically states this.

 

[Victor Bazarov]

It's not that I doubt you, but it would be nice to see the part of the
standard that specifically states this.

The Standard does not specifically state what "is not undefined
behavior". It only states what *is*. In the section about 'delete'
there are several scenarios that are specified to have undefined
behavior and 'delete this' is not one of them. Of course, there can be
a situation in which 'delete this' can fit. For instance, if the
pointer 'this' was not obtained by a previous call to 'new'...

V

 

[Marcel Müller]

"delete this" is seen in places, and, beside intrusive refcounting, I
am yet to see a reason why it is a good idea. In your case, ownership
of the object is effectively taken by whoever is going to call
OnCompleted. So that somebody can do

p->OnCompleted();
delete p;

No, OnCompleted is private. So the class owns itself.

Basically the class requests several information from different places
asynchronously and registers completion call-backs. Once sufficient
information is available (not necessarily all) the remaining
subscriptions are canceled and the original task, that has been
suspended because of missing dependencies, is rescheduled to a thread
pool. This lets me deal with complex dependency trees with no need to
keep track of all suspended tasks and their dependencies at a central
location, with all its concurrency issues. The required information to
control all the thousands of dependencies is simply held in the
structure of the OnChange event subscriptions.

or better yet

auto_ptr<ReshceduleWorker>(p)->OnCompleted();

This is not an option unless the auto_ptr is a member of
RescheduleWorker which obviously is equivalent to delete this.

and "delete this" should disappear.

"delete this" is usually a sign of underdesigned heap object
ownership.

Well, a better solution for the above scenario would be appreciated.


Marcel

 

[Goran]

No, OnCompleted is private. So the class owns itself.

I would say that's the wrong ownership decision. First off, as others
have pointed out, you can instantiate your class in static or
automatic storage, in which case an instance cannot own itself (owner
is the storage). You can easily circumvent this by making sure that
you can only create instances of your class in dynamic storage, though
(you should do that).

To go back back to basics, there's three storage types: static,
automatic (they both "own", end of). As for dynamic storage (a.k.a.
heap), it's the programmer (code as a whole) who owns the object. I
greatly dislike the idea of the instance owning itself, because it's
nigh impossible for it do do anything like that. You're not showing
what "register some callbacks in dependency list" means, but clearly
there's some other state in your program that uses your object
somehow.

Basically the class requests several information from different places
asynchronously and registers completion call-backs. Once sufficient
information is available (not necessarily all) the remaining
subscriptions are canceled and the original task, that has been
suspended because of missing dependencies, is rescheduled to a thread
pool. This lets me deal with complex dependency trees with no need to
keep track of all suspended tasks and their dependencies at a central
location, with all its concurrency issues. The required information to
control all the thousands of dependencies is simply held in the
structure of the OnChange event subscriptions.

This is too convoluted for my small brain, and I also believe that
there's clearly crucial information missing, so I can't comment. I
know this, though: I used shared_ptr / weak_ptr with great success
(meaning: simple code) in multithreaded situations: shared_ptr is used
in whatever "central" location. All else (e.g. your callback
registrations), use weak_ptr. When object is "completed", shared_ptr
is removed from the program, and all subsequent weak_ptr uses see a
null pointer (weak_ptr.lock() yields an empty shared_ptr) and do
nothing.

In other words, uncertain lifetime is handled through shared_ptr and
weak_ptr.

This is not an option unless the auto_ptr is a member of
RescheduleWorker which obviously is equivalent to delete this.

But, but... auto_ptr and RescheduleWorker are completely orthogonal.
No membership is involved.

HTH,

Goran.

 

[Noah Roberts]

The Standard does not specifically state what "is not undefined
behavior".  It only states what *is*.  In the section about 'delete'
there are several scenarios that are specified to have undefined
behavior and 'delete this' is not one of them.  Of course, there can be
a situation in which 'delete this' can fit.  For instance, if the
pointer 'this' was not obtained by a previous call to 'new'...

As I recall actually, anything not defined by the standard is defined
by the standard as "undefined" by default.

Although an explicit statement that "delete this" is legal isn't
available in the standard, I believe it's sufficiently specified by
the specified behavior of pointers (this being specifically defined as
one), the delete operator, and how the delete operator works on
pointers.

Calling delete on the "my_ptr" pointer isn't explicitly allowed either
and just like "this" it doesn't need to be.

 

[Daryle Walker]

  It's still unclear to me if 'delete this' is defined or undefined
behavior according to the standard. I wouldn't be surprised if it was
undefined because I'm assuming the standard does not want to forbid
a compiler from putting some kind of "cleaning code" at the end of a
member function if it needs to do that for whatever reason. If this
"cleaning code" accesses the object pointed by 'this', we have a problem.

[TRUNCATE]

Using "delete this" is legal. You just need to take the proper
precautions:

* Obviously, you can't make any more (de)references to the just killed
object.
* Also obviously, the object must have been made with the standard
single-object "new". Specifically, it can't be created by:
* * array new
* * an array element
* * a non-static class member
* * a base class, virtual or not
* * a global
* * * in a namespace, including the root-level one or unnamed ones
* * * a static class member
* * * a static within a function or member function
* You should block improper allocations by making the destructor and
constructors private.
* Your objects need to be created with a friend function or a public
static member function.

Daryle W.

 

[Daryle Walker]

I have a class that destroys itself after it has done its job. Something
like a thread, but without a thread.

// Some Object
class MyObject;

// Schedule a task for obj in the Worker Queue.
void ScheduleObject(MyObject* obj);

class RescheduleWorker
{private:
intrusive_ptr<MyObject> Object;
vector<intrusive_ptr<MyObject> > Dependencies;
public:
RescheduleWorker(MyObject& obj,
const vector<intrusive_ptr<MyObject> >& dependencies)
: Object(&obj), Dependencies(dependencies)
{ // register some callbacks in the dependency list
}
private:
~RescheduleWorker() {}
// Called by some internal observer functions exactly once
// when a complex dependency condition is true.
void OnCompleted()
{ ScheduleObject(Object.get());
delete this;
}

};

Constructors and destructors ignore how the object's bit bucket was
allocated, so if you are restricting methods of deletion, then you
must restrict methods of creation so others don't allocate
recklessly. I also threw in a way of listing the dependencies
directly, without having to make a std::vector first.

// I've never used C++11 myself, mostly guessing
class Rescheduleworker
{
public:
template <typename... Args>
static void create_and_attach( MyObject &main_object, Args...
&objects )
{
std::unique_ptr<Rescheduleworker> w{ new
Rescheduleworker{main_object, objects} };

// Any other code goes here. Only if there isn't any you can
// replace the entire function with a single "new
// Rescheduleworker{main_objects, objects}" line. The way I
// have it here makes it safe if the other code throws.

w.release();
}

private:
using ObjectPtr = intrusive_ptr<MyObject>;

ObjectPtr Object;
std::vector<ObjectPtr> Dependencies;

FillDependencies()
{}

template <typename... Args>
FillDependencies( MyObject &dependency, Args... &dependencies )
{
this->Dependencies.push_back( &dependency );
this->FillDependencies( dependencies );
}

template <typename... Args>
Rescheduleworker( MyObject &obj, Args... &dependencies )
: Object{ &obj }, Dependencies{}
{
this->Dependencies.reserve( sizeof...(dependencies) );
this->FillDependencies( dependencies );

// Put any other setup here.

// Your code may vary; but something like this must be set up,
// where the completion routine will be called, which
// deallocates this object.
Something( obj ).SetSomeMethod( std::bind(this,
&Rescheduleworker::OnCompleted) );
}

~Rescheduleworker() = default;

void OnCompleted()
{
ScheduleObject( this->Object.get() );
delete this;
}
};

// usage:
new RescheduleWorker(obj, dependencies);

// Now:
// (Of course, now it does NOT work if you already have the
dependencies in
// a vector or other container. We would need another
"create_and_attach"
// and constructor (template) overloads for vectors (and/or
iterators).)
Rescheduleworker::create_and_attach( obj, dependency1, ...,
dependencyN );

The latter looks a bit strange, because the pointer returned by new is
immediately discarded. Is this a common pattern? How is it called?

This seems to be a variant of the factory pattern, except that the
returned pointer is never used.

Daryle W.