Should I convert shared_ptr to weak_ptr when passed to a method?

there are already a couple of questions regarding this topic, but I am still not sure what to do: Our codebase uses shared_ptr at many places. I have to admit that we did not define ownership clearly when writing it.

We have some methods like

 void doSomething(shared_ptr<MyClass> ptr)
 {
      //doSomething() is a member function of a class, but usually won't store the ptr
      ptr->foo();
      ...
 }

After having discovered the first (indirect) circular dependencies I would like to correct the mistakes in our design. But I'm not exactly sure how. Is there any benefit in changing the method from above to

 void doSomething(weak_ptr<MyClass> ptr)
 {
      shared_ptr<MyClass> ptrShared = ptr.lock();
      ptrShared->foo();
      ...
 }

?

I am also confused because some people say (including the Google Style guide) that in the first place it's important to get ownership correct (which would probably mean introduction of many weak_ptrs, e.g. in the example with the methods above, but also for many member variables that we have). Others say (see links below) that you should use weak_ptr to break cyclic dependencies. However, detecting them is not always easy, so I wonder if I really should use shared_ptr until I run into problems (and realize them), and then fix them??

Thanks for your thoughts!

See also

• shared_ptr and weak_ptr differences
• boost::shared_ptr开发者_JS百科 cycle break with weak_ptr
• boost, shared ptr Vs weak ptr? Which to use when?

---

We did not define ownership clearly.

You need to clearly define who owns what. There's no other way to solve this. Arbitrarily swapping out some uses of shared_ptr with weak_ptr won't make things better.

---

There is no benefit in changing your design above from shared_ptr to weak_ptr. Getting ownership right is not about using weak_ptrs, it's about managing who stores the shared_ptr for any significant length of time. If I pass a shared_ptr to a method, assuming I don't store that shared_ptr into a field in my object as part of that method, I haven't changed who owns that data.

In my experience the only reason for using weak_ptr is when you absolutely must have a cycle of pointers and you need to break that cycle. But first you should consider if you can modify your design to eliminate the cycle.

I usually discourage mixing shared_ptr's and raw pointers. It inevitably happens (though it probably shouldn't) that a raw pointer needs to be passed to a function that takes a shared_ptr of that type. A weak_ptr can be safely converted to a shared_ptr, with a raw pointer you're out of luck. Even worse, a developer inexperienced with shared_ptr's may create a new shared_ptr from that raw pointer and pass it to the function, causing that pointer to be deleted when the function returns. (I actually had to fix this bug in production code, so yes it does happen :) )

---

It sounds like you have a design problem. shared_ptr provides a simple to use implementation for specific design solutions, but it (nor anything else) can replace the design. Until you have determined what the actual lifetime of each type of object should be, you shouldn't be using shared_ptr. Once you've done that, most of the shared_ptr/weak_ptr issues should disappear.

If, having done that, and determined that the lifetime of some objects does depend on that of other objects, and there are cycles in this dependency, you have to determine (at the design level, again) how to manage those cycles---it's quite possible, for example, that in those cases, shared_ptr isn't the correct solution, or that many of the pointers involved are just for navigation, and should be raw pointers.

At any rate, the answer to your question resides at the design level. If you have to ask it when coding, then it's time to go back to design.

---

Some people are right: you should at first have very clear picture about objects' ownership in your project.

shared_ptrs are shared, i.e. "owned by community". That might or might not be desirable. So I would advise to define ownership model and then to not abuse shared_ptr semantics and use plain pointers whenever ownership should not be "shared" more.

Using weak_ptrs would mask the problem further rather than fix it.