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
    intrusive_ptr<MyObject> Object;
    vector<intrusive_ptr<MyObject> > Dependencies;
    RescheduleWorker(MyObject& obj,
    const vector<intrusive_ptr<MyObject> >& dependencies)
    : Object(&obj), Dependencies(dependencies)
    { // register some callbacks in the dependency list
    ~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 Müller, Nov 9, 2011
    1. Advertisements

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

    Victor Bazarov, Nov 9, 2011
    1. Advertisements

  3. 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
  4. Marcel Müller

    Werner Guest

    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(
    const vector<intrusive_ptr<MyObject> >& )"

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


    Werner, Nov 10, 2011
  5. Marcel Müller

    Krice Guest

    Check out "local instance". It does what you try to do in proper way.
    Krice, Nov 10, 2011
  6. Marcel Müller

    Werner Guest

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

    Kind Regards,

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

    Goran Guest

    "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

    delete p;

    or better yet


    and "delete this" should disappear.

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

    Goran, Nov 10, 2011
  8. 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
  9. A good point. It makes things more clear. Especially it prevents from
    writing RescheduleWorker(obj, dependencies) (without new) accidentally.

    Marcel Müller, Nov 10, 2011
  10. 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
  11. 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'...

    Victor Bazarov, Nov 10, 2011
  12. 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.

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

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

    Marcel Müller, Nov 10, 2011
  13. Marcel Müller

    Goran Guest

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

    In other words, uncertain lifetime is handled through shared_ptr and
    But, but... auto_ptr and RescheduleWorker are completely orthogonal.
    No membership is involved.


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

    Noah Roberts Guest

    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

    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
  15. [TRUNCATE]

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

    * Obviously, you can't make any more (de)references to the just killed
    * 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
  16. 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
    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.


    using ObjectPtr = intrusive_ptr<MyObject>;

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


    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;
    // Now:
    // (Of course, now it does NOT work if you already have the
    dependencies in
    // a vector or other container. We would need another
    // and constructor (template) overloads for vectors (and/or
    Rescheduleworker::create_and_attach( obj, dependency1, ...,
    dependencyN );
    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
    1. Advertisements

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.