11

I've been away from serious C++ for about ten years. I'm coming back in to the fold and am currently working on a project to fully familiarize myself with C++11. I'm having a bit of an existential crisis about how to best pass std::shared_ptr's around.

For a simple example, take the following setup:

class ServiceB {
public:
    ServiceB() {}
};

class ServiceA {
public:
    ServiceA(std::shared_ptr<ServiceB>& serviceB)
        : _serviceB(serviceB) {

    }

private:
    std::shared_ptr<ServiceB> _serviceB;
};

class Root {
public:
    Root()
        : _serviceB(std::shared_ptr<ServiceB>(new ServiceB())),
        _serviceA(std::unique_ptr<ServiceA>(new ServiceA(_serviceB))) {

    }

private:
    std::shared_ptr<ServiceB> _serviceB;
    std::unique_ptr<ServiceA> _serviceA;
};

Notice here that ServiceA requires a reference to ServiceB. I'd like to keep that reference tied up in a shared_ptr. Am I okay to do what I did here, which is simply pass the shared_ptr down as a reference and let the std::shared_ptr copy constructor do the work for me? Does this properly increment the reference count on the shared_ptr?

If this is not the best way to do this, what is the common "best practice" for passing around std::shared_ptr's?

Matt Holmes
  • 1,055
  • 1
  • 8
  • 22
  • 7
    Take a look at [Guru of the Week #91](http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/) which is about exactly this subject. – Casey Jan 03 '14 at 17:39
  • 3
    The C++11 rule of thumb "Pass by value if you are going to make a copy anyway" applies to `shared_ptr` as much as to every other type. – Casey Jan 03 '14 at 17:41
  • 1
    Also use make_shared instead of constructing shared_ptr by hand. It saves you one allocation and prevents memory leaks in certain scenarios. http://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared – aryan Jan 03 '14 at 17:52
  • @aryjczyk I finally took the time to look up how make_shared/make_unique actually work, so thanks for the tip. I was trying to use them like "std::make_shared(existingPtr)" rather than realizing they actually construct my object using some template magic. I still have one case where I have to do std::unique_ptr because I have an existing pointer to wrap, but every where else I am using make_shared/make_unique. – Matt Holmes Jan 03 '14 at 18:33
  • related http://stackoverflow.com/questions/3310737/shared-ptr-by-reference-or-by-value – Cubbi Jan 03 '14 at 19:01
  • @Cubbi: Unfortunately it looks like both those talks were given in 2010, before move semantics were terribly common place. Given move semantics their assertion that passing by value is almost "never correct" is out dated, as Herb himself states in later talks. Thanks for the links though, plenty of good stuff in those videos. – Matt Holmes Jan 03 '14 at 19:10

3 Answers3

16

You should pass around shared pointers exactly as you pass around other objects. If you need to store a copy (of the shared pointer, not the pointed at object), pass by value, and move to its destination.

ServiceA(std::shared_ptr<ServiceB> serviceB)
    : _serviceB(std::move(serviceB)) {}

Alternatively, if you don't mind writing two constructors, you can save a tiny bit of performance (one call to the shared pointer's the move constructor) by writing one which takes a const reference and copies it, and one which takes an r-value reference, and moves it.

ServiceA(std::shared_ptr<ServiceB> const& serviceB)
    : _serviceB(serviceB) {}

ServiceA(std::shared_ptr<ServiceB> && serviceB)
    : _serviceB(std::move(serviceB) {}
Benjamin Lindley
  • 101,917
  • 9
  • 204
  • 274
  • Copying of a shared_ptr adjusts the reference count (with possible locking or atomic operations), and so might moving. So the pass by value (one, two or three updates of ref count) can't be very much faster than pass by ref to const and copying (one update). As I see it. Not sure if this was what you're talking about though. :-) – Cheers and hth. - Alf Jan 03 '14 at 17:47
  • 2
    @Cheersandhth.-Alf: If move constructing a shared pointer adjusts the reference count, is that not a sign of a severely incompetent implementation? I can't imagine why it would ever be necessary. – Benjamin Lindley Jan 03 '14 at 17:51
  • no. the choice (for the move constructor) of zeroing the original or increasing the reference count and keeping the original, depends ideally on what's fastest, and that's not obvious at all. doing the slowest thing, or not adjusting the ref count on some ideaqlistic or "can't imagine" grounds, would however, as you say, be a sign of an incompetent's implementation. – Cheers and hth. - Alf Jan 03 '14 at 17:54
  • 1
    @Alf Zeroing avoids needing to synchronise, which is likely to be a win. – Alan Stokes Jan 03 '14 at 17:57
  • @AlanStokes: assuming that it's so, it still comes in addition to the copying and single ref count update (of pass by ref to const). :-) however the assumption is dubious. – Cheers and hth. - Alf Jan 03 '14 at 17:59
  • @Cheersandhth.-Alf: The other issue is that pass by const reference will not always be just one update to the ref count. It's only one update if an l-value is passed in. But if an r-value is passed in, it's an unnecessary copy and destruction, resulting in 3 updates to the reference count. That's why this idiom exists in the first place. – Benjamin Lindley Jan 03 '14 at 18:02
  • @BenjaminLindley: optimizing the rvalue case is what the pass by rvalue ref overload is about. it's so no matter what scheme one employs. – Cheers and hth. - Alf Jan 03 '14 at 18:03
  • @Cheersandhth.-Alf: That's why this answer comes in two parts. The top part assumes you only want to write one constructor. In which case, "by value, then move" is to be preferred, in my opinion. – Benjamin Lindley Jan 03 '14 at 18:13
  • Both the top answers were good, but I found this one more complete for the added move constructor implementation, so I've marked this one. – Matt Holmes Jan 03 '14 at 18:14
  • @BenjaminLindley: if "you only want to write one constructor" then for lvalue argument the pass by value can't be faster, and is most likely a bit slower. and it doesn't work as a general convention. so there's apparently no good reason for it other than to be chic, doing the latest stuff no matter whether it's meaningful in case at hand. – Cheers and hth. - Alf Jan 03 '14 at 18:18
  • @Cheersandhth.-Alf: You're right, it can't be faster for l-values. I never said it could. The constructor is meant to be the best compromise between being highly performant for *both* r-values and l-values. It relies on the general assumption that moving is cheap, and copying is (more) expensive. Not always a correct assumption, but in the case of `shared_ptr`, it is. – Benjamin Lindley Jan 03 '14 at 18:20
  • @BenjaminLindley: as a general convention it's quite inefficient. consider passing a shared_ptr down 5 function calls. with pass by value that's 5 updates of the ref count, involving 5 thread synchronizations (likely via atomics but still), while with pass by reference to const it's 0 updates, 0 synchs. Since we have already dispensed with the case at hand (lvalue, pass by value ungood for that), and now also with the general case where the pointer is most often not stored away but just passed around, I fail to see any advantage other than doing the blindly-perceived-as-modern thing. – Cheers and hth. - Alf Jan 03 '14 at 19:14
  • 2
    @Cheersandhth.-Alf: My answer only applies *"If you need to store a copy"* -- It's right there, in the second sentence. If you need to store 5 copies, then you will have to make 5 copies, and the reference count will be incremented 5 times. Taking the parameter in by const reference will not change that fact. If you *don't* need any copies, then this idiom is not applicable to your case, and you should probably take the argument by reference to const (or non-const, depending upon your needs). This is fun. – Benjamin Lindley Jan 03 '14 at 19:57
  • i.e. your answer applies not to the OP's case at hand (passing an lvalue to copy), nor does it apply to the general case (passing lvalues around), but to a subset of cases that are like the OP's case except for the crucial details in the rationale (passing a mix of rvalues and values, transparently to the caller). and for some reason you think that's "fun". i must cease to try to make sense of SO. – Cheers and hth. - Alf Jan 03 '14 at 20:39
  • @Cheersandhth.-Alf: I'm sorry, you're not making any sense to me now. In the OP's case, he *is* storing a copy of the shared pointer. So, as far as I can tell, my answer very much does apply to the OP's case, *exactly*. And the fun thing I was referring to was this back and forth between you and I. – Benjamin Lindley Jan 03 '14 at 20:43
  • @Cheersandhth.-Alf: Nevermind, I understand what you are saying now. You are referring to the `Root` class, passing its l-value `shared_ptr` to the constructor of the other object. Okay, you're right, it doesn't help in this specific use case. But it doesn't make any sense to design the class constructor of the `ServiceA` class just so it is optimized (narrowly) for the way this other class (`Root`) uses it, and ignore how other classes and functions might use it. – Benjamin Lindley Jan 03 '14 at 20:54
  • @Cheersandhth.-Alf: Finally, it *does* apply to the general case, you just made the wrong generalization. The general case, in this instance, is not "passing l-values around", it is "passing values (r or l) to store copies of". – Benjamin Lindley Jan 03 '14 at 21:05
  • Hm, that may well be where our opinions differ. I see those two classes as very tightly coupled, where it doesn't make sense to do anything else than exactly what's shown. It's just the pros and cons of various ways of doing that, that is at issue. Okay. Over to ... something else. .-) – Cheers and hth. - Alf Jan 03 '14 at 22:10
2

Either pass by value (compilers are pretty good at eliding copies) or by const reference - non-const reference as you have makes it look like you intend to modify the parameter.

Also for new code consider using unique_ptr for parameters and return values where it makes sense, or where the sharing isn't part of the contract. (You can make a shared_ptr from a unique_ptr, but not vice versa.)

Alan Stokes
  • 18,815
  • 3
  • 45
  • 64
  • `unique_ptr` is not well suited for a general reference, since it has ownership and destroys the referent. – Cheers and hth. - Alf Jan 03 '14 at 17:51
  • @Cheersandhth.-Alf You missed 'where the sharing isn't part of the contract' –  Jan 03 '14 at 17:52
  • 1
    @Alf Sure - but a lot of old code that used `shared_ptr` (as a safe way to pass ownership around) would nowadays be better written with `unique_ptr`. – Alan Stokes Jan 03 '14 at 17:53
2

pass by reference to non-const when you intend to modify the actual argument.

pass by reference to const when you don't intend that.

the reference to const is technically just a micro-optimization for shared_ptr, but it doesn't harm, it's the common convention for class type arguments, and it just can shave off some nano-seconds.

a different matter, since C uses prepended underscore for implementation names it's best to avoid that in C++, so instead of prepending an underscore, place it at the end. e.g. that's the convention in the boost library. or just use some other convention.

Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331