14

I work with a codebase that was partially implemented by someone who was in love with overly complex solutions to simple problems (e.g. template classes with two parameters that were only ever instantiated for one pair of types). One thing she did was to create objects in a smart pointer, and then have the object store a weak pointer to itself.

class MyClass {
    //...
    boost::weak_ptr<MyClass> m_self;
    //...
};

boost::shared_ptr<MyClass>
Factory::Factory::Factory::CreateMyClass() {
    boost::shared_ptr<MyClass> obj(new MyClass(...));
    boost::weak_ptr<MyClass> p(obj);
    obj->storeSelfPointer(p);
    return obj;
}

The class then proceeds to use m_self by locking it and passing around the resulting shared pointer.

For the life of me, I cannot fathom what she was trying to accomplish. Is there some pattern or idea that would explain this implementation? It looks to me like this is completely pointless and I'd like to refactor it away.

EDIT: I should mention that none of the places that use the resulting smart pointer obtained from locking m_self actually retain the smart pointer.

James Davidoff
  • 275
  • 2
  • 6

1 Answers1

19

A possible use of this "design" could be to use m_self.lock() to generate shared pointers from this.

If you remove this weak pointer member, the reference count hold by the generated shared pointer from this would be incorrect.

It achieves the same than std::enable_shared_from_this, interestingly enough, cppreference.com mentions this design :

A common implementation for enable_shared_from_this is to hold a weak reference (such as std::weak_ptr) to this. The constructors of std::shared_ptr detect the presence of an enable_shared_from_this base and assign the newly created std::shared_ptr to the internally stored weak reference

And the C++ standard, section § 20.8.2.4 10 , mention the same possible implementation :

The shared_ptr constructors that create unique pointers can detect the presence of an enable_shared_- from_this base and assign the newly created shared_ptr to its __weak_this member

Possible Refactoring :

  • If you are using C++11, you can remove the std::weak_ptr member, and publicly inherits from std::enable_shared_from_this<T>. You should retrieve a shared pointer from this by calling shared_from_this().

  • If you are not using C++11 but can use boost, use boost::enable_shared_from_this, see the boost documentation. You should retrieve a shared pointer from this by calling shared_from_this().

  • If you are not using C++11, and can't use boost, you can bring the proposed implementation of the standard to your code base, it is short enough :

Code : (copied from § 20.8.2.4 - 11, remove leading underscores, and you probably want to rename it)

template<class T> class enable_shared_from_this {
    private:
     weak_ptr<T> __weak_this;
    protected:
     constexpr enable_shared_from_this() : __weak_this() { }
     enable_shared_from_this(enable_shared_from_this const &) { }
     enable_shared_from_this& operator=(enable_shared_from_this const &) { return *this; }
     ~enable_shared_from_this() { }
    public:
     shared_ptr<T> shared_from_this() { return shared_ptr<T>(__weak_this); }
     shared_ptr<T const> shared_from_this() const { return shared_ptr<T const>(__weak_this); }
};

And use shared_from_this() to make a shared pointer. If you do copy this code, note that constructing shared pointers from this by other means would not work. The shared pointers constructors need to be modified (as explain by the standard quote above).

quantdev
  • 23,517
  • 5
  • 55
  • 88
  • 1
    That's not official documentation – Ben Voigt Jun 26 '14 at 00:41
  • 1
    True. Actually the standard shows the exact same possible implementation. – quantdev Jun 26 '14 at 00:44
  • Yup I was trying to confirm that, but PDF search wasn't working. I see it now. – Ben Voigt Jun 26 '14 at 00:45
  • The note accompanying that example is kinda important...how would the library `shared_ptr` know the existence of your `enable_shared_from_this` clone let alone access its `__weak_this`? – T.C. Jun 26 '14 at 01:10
  • @T.C. : Yes, you would not have the features exposed in the standard, but by using `shared_from_this()`, the OP would keep its current behavior. Creating shared_pointer from this by other means would still not work without modifying `shared_pointer` ctors, Ill edit.. – quantdev Jun 26 '14 at 01:19
  • This explanation smells right. I'll give you the mark, and I'm going to give her bonus points for implementing a complicated mess and then not using it; after locking `m_self`, the resulting `shared_ptr` is passed around to various functions by const ref and never copied or stored. In other words, it is used as a very expensive `this`. – James Davidoff Jun 26 '14 at 01:25