What kind of pattern is this?

Discussion in 'C++' started by Marcel Müller, Nov 9, 2011.

  1. 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
    Marcel Müller, Nov 9, 2011
    #1
    1. Advertising

  2. On 11/9/2011 2:54 PM, Marcel Müller wrote:
    > 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
    --
    I do not respond to top-posted replies, please don't ask
    Victor Bazarov, Nov 9, 2011
    #2
    1. Advertising

  3. On 11/9/11 3:57 PM, Victor Bazarov wrote:
    > On 11/9/2011 2:54 PM, Marcel Müller wrote:
    >>

    > 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).
    Richard Damon, Nov 10, 2011
    #3
  4. Marcel Müller

    Werner Guest

    On Nov 9, 9:54 pm, Marcel Müller <> wrote:
    > 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
    Werner, Nov 10, 2011
    #4
  5. Marcel Müller

    Krice Guest

    On 9 marras, 21:54, Marcel Müller <>
    wrote:
    > 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.
    Krice, Nov 10, 2011
    #5
  6. Marcel Müller

    Werner Guest

    On Nov 10, 9:13 am, Krice <> wrote:
    > On 9 marras, 21:54, Marcel Müller <>
    > wrote:
    >
    > > 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.


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

    Kind Regards,

    Werner
    Werner, Nov 10, 2011
    #6
  7. Marcel Müller

    Goran Guest

    On Nov 9, 8:54 pm, Marcel Müller <> wrote:
    > 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.
    Goran, Nov 10, 2011
    #7
  8. Goran <> wrote:
    > "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.)
    Juha Nieminen, Nov 10, 2011
    #8
  9. On 10.11.2011 07:51, Werner wrote:
    > 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
    Marcel Müller, Nov 10, 2011
    #9
  10. Leigh Johnston <> wrote:
    > '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.
    Juha Nieminen, Nov 10, 2011
    #10
  11. On 11/10/2011 4:03 PM, Juha Nieminen wrote:
    > Leigh Johnston<> wrote:
    >> '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.


    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
    --
    I do not respond to top-posted replies, please don't ask
    Victor Bazarov, Nov 10, 2011
    #11
  12. On 10.11.2011 11:55, Goran wrote:
    > "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
    Marcel Müller, Nov 10, 2011
    #12
  13. Marcel Müller

    Goran Guest

    On Nov 11, 12:59 am, Marcel Müller <>
    wrote:
    > On 10.11.2011 11:55, Goran wrote:
    >
    > > "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.


    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.

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


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

    HTH,

    Goran.
    Goran, Nov 11, 2011
    #13
  14. Marcel Müller

    Noah Roberts Guest

    On Nov 10, 1:28 pm, Victor Bazarov <> wrote:
    > On 11/10/2011 4:03 PM, Juha Nieminen wrote:
    >
    > > Leigh Johnston<>  wrote:
    > >> '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.

    >
    > 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.
    Noah Roberts, Nov 11, 2011
    #14
  15. On Nov 10, 7:25 am, Juha Nieminen <> wrote:
    > Goran <> wrote:
    > > "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.

    [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, Nov 13, 2011
    #15
  16. On Nov 9, 2:54 pm, Marcel Müller <> wrote:
    > 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.
    Daryle Walker, Nov 13, 2011
    #16
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Replies:
    17
    Views:
    1,832
    Chris Uppal
    Nov 16, 2005
  2. sunny
    Replies:
    1
    Views:
    446
    Salt_Peter
    Dec 7, 2006
  3. Giff
    Replies:
    6
    Views:
    324
  4. Pallav singh
    Replies:
    0
    Views:
    341
    Pallav singh
    Jan 22, 2012
  5. Pallav singh
    Replies:
    0
    Views:
    387
    Pallav singh
    Jan 22, 2012
Loading...

Share This Page