The best way to protect against multiple shared_ptr to the same object

Question

Sending the same pointer to two different shared_ptr is bad, it causes a double release, for example:

 int* p = new int; std::shared_ptr<int> p1(p); std::shared_ptr<int> p2(p); // BAD 

You can achieve the same goal with std::enable_shared_from_this :

 class Good: public std::enable_shared_from_this<Good> { public: std::shared_ptr<Good> getptr() { return shared_from_this(); } }; int main() { std::shared_ptr<Good> gp1(new Good); std::shared_ptr<Good> gp2 = gp1->getptr(); } 

But this does not protect:

 class Good: public std::enable_shared_from_this<Good> { public: std::shared_ptr<Good> getptr() { return shared_from_this(); } }; int main() { Good* p = new Good; std::shared_ptr<Good> gp3(p); std::shared_ptr<Good> gp4(p); // BAD } 

which can become a problem if you have code like this:

 void Function(std::shared_ptr<Good> p) { std::cout << p.use_count() << '\n'; } int main() { Good* p = new Good; std::shared_ptr<Good> p1(p); Function(p); // BAD } 

Why should I use a regular pointer when there are smart pointers? Because in performance-critical code (or for convenience), the overhead of shared_ptr or weak_ptr is undesirable.

To prevent this error, I did:

 class CResource : public shared_ptr<Good> { public: CResource() { } CResource(std::shared_ptr<CBaseControl> c) : CResource(c) { } private: CResource(CBaseControl* p) { } }; void Function(CResource p) { std::cout << p.use_count() << '\n'; } int main() { Good* p = new Good; CResource p1(std::shared_ptr<Good>(p)); Function(p); // Error } 

This will cause a compiler error if someone tries to call Function pointer instead of shared_ptr . This does not stop someone from declaring a void Function(std::shared_ptr p) , although, but I think it's unlikely.

Is it really bad? Is there a better way to do this?

Answer 1

The solution is simple: first of all, never have your own memory with a pointer. This template:

 int* p = new int; std::shared_ptr<int> p1(p); std::shared_ptr<int> p2(p); // BAD 

It just should not exist. Eliminate it from the code base. The only places where new is legal in C ++ 11 are the argument in invoking the constructor on the smart pointer (or in material with a very low level).

those. there is such a code:

 std::shared_ptr<int> p1(new int); 

Please note that it is sometimes useful to use raw pointers in a performance-critical codec (but a question with an identifier, even in most codecs), did you profile? Is smart pointer bottleneck here? If so, do you really need to exchange property?). But if you use raw pointers, don't let them have memory. Either specify automatic storage (there is no need to manage resources), or use unique_ptr and access the raw pointer through its get function.

Answer 2

This example does not even compile:

 void Function(std::shared_ptr<Good> p) { std::cout << p.use_count() << '\n'; } int main() { Good* p = new Good; std::shared_ptr<Good> p1(p); Function(p); // BAD } 

shared_ptr has an explicit constructor to stop this.

To complete this compilation you need to write:

  Function( std::shared_ptr<Good>(p) ); 

which is obviously wrong, and if someone is going to make this mistake, they are just as likely:

  Function( CResource(std::shared_ptr<Good>(p)) ); 

So why did you bother to write CResource ? What does he add?

To deploy on Konrad Rudolph an excellent answer:

The answer to your question on how to avoid problems is to follow RAII and complete it completely.

We will ignore the example, which does not even compile, and do not look at it above:

 Good* p = new Good; std::shared_ptr<Good> gp3(p); std::shared_ptr<Good> gp4(p); // BAD 

This code cannot follow the RAII idiom. You get the resource:

  Good* p = new Good; 

But do not initialize the type of RAII. Bad .

Then you initialize the object using some existing resource:

  std::shared_ptr<Good> gp3(p); 

This is also a bad . You must initialize the RAII type at the same time as acquiring the resource, and not separately (not even separated by just one line).

Then you repeat the same error:

  std::shared_ptr<Good> gp4(p); // BAD 

You marked this line as "BAD", but in fact the previous two lines were just as bad. The third line will cause undefined behavior, but the first two lines allowed this error to penetrate when it was supposed to get complicated. If you never had a Good* suspension, then you could not use it to initialize gp4 , you would need to say shared_ptr<Good> gp4(gp3.get()) , which is clearly wrong, no one will do that.

The rule is pretty simple: don't use a raw pointer and put it in shared_ptr . A raw pointer that you did not allocate is not a collection of resources, so do not use it to initialize. The same thing happens inside Function , it should not take a raw pointer and use it to initialize shared_ptr , which becomes the property of the type.

This is C ++, therefore it is impossible to protect against all the wrong ways of writing code, you cannot stop sufficiently motivated idiots from shooting in the foot, but all the recommendations and tools exist to prevent this if you follow the recommendations.

You have an interface in which it forces you to transfer ownership by passing a raw pointer, try replacing the interface so that it uses unique_ptr to transfer ownership, or if this is completely impossible to do then try wrapping the interface in a more secure version using unique_ptr to transfer ownership , and only as a last resort use a dangerous interface, but write it down very explicitly to transfer ownership.