10

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

Community
  • 1
  • 1
Philipp
  • 11,549
  • 8
  • 66
  • 126
  • Would shared_ptr &ptr be better? – DanDan Mar 23 '11 at 16:01
  • 1
    "detecting them is not always easy" - then really you need to add more steps to your design process. Light and dark lines on your class relationship diagrams, or whatever it takes. Also you can consider breaking cycles with raw pointers rather than weak pointers, for most asymmetrical relationships where the "owner" will never be destroyed before all "non-owners". – Steve Jessop Mar 23 '11 at 16:01
  • 1
    @DanDan: I don't think so, see http://stackoverflow.com/questions/327573/c-passing-references-to-boostshared-ptr – Philipp Mar 23 '11 at 16:03
  • 1
    Some of the comments in that thread weren't particularly valid. Whether to pass a shared_ptr by reference or not obeys more or less the same rules as passing anything by reference; in this regard, there's nothing special about shared_ptr. – James Kanze Mar 23 '11 at 17:02

4 Answers4

6

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.

James McNellis
  • 348,265
  • 75
  • 913
  • 977
  • ok, I think this shouldn't be too hard as the design is (not yet :-)) too bad, I guess. However the question remains: should I introduce weak_ptrs only where I detect circles (if there are any more) or everywhere where the object/method doesn't have ownership? – Philipp Mar 23 '11 at 17:47
  • 1
    @Phillip - If you look at the example you gave, there is no difference in behaviour between passing a weak_ptr and a shared_ptr. To use the weak_ptr, you must first create a shared_ptr from it, which is effectively the same as having passed a shared_ptr in the first place. – Nathanael Mar 23 '11 at 18:46
5

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

Nathanael
  • 1,782
  • 17
  • 25
  • "_It inevitably happens_ (...)" This explanation is one of the very few articulate justification for `weak_ptr` I have seen on SO (or elsewhere). Thank you. – curiousguy Jul 22 '12 at 14:35
3

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.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
2

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.

Alexander Poluektov
  • 7,844
  • 1
  • 28
  • 32