107

If I declare an object wrapped in a shared pointer:

std::shared_ptr<myClass> myClassObject(new myClass());

then I wanted to pass it as an argument to a method:

DoSomething(myClassObject);

//the called method
void DoSomething(std::shared_ptr<myClass> arg1)
{
   arg1->someField = 4;
}

Does the above simply increment the shared_pt's reference count and everything is cool? Or does it leave a dangling pointer?

Are you still supposed to do this?:

DoSomething(myClassObject.Get());

void DoSomething(std::shared_ptr<myClass>* arg1)
{
   (*arg1)->someField = 4;
}

I think that the 2nd way may be more efficient because it only has to copy 1 address (as opposed to the whole smart pointer), but the 1st way seems more readable and I do not anticipate pushing performance limits. I just want to make sure there's not something dangerous about it.

Thank you.

R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
Steve H
  • 5,479
  • 4
  • 20
  • 26
  • 16
    `const std::shared_ptr& arg1` – Captain Obvlious May 31 '12 at 02:01
  • 3
    The second way is broken, the first is idiomatic if you actually need your function to share ownership. But, does `DoSomething` really need to share ownership? It looks like it should just take a reference instead... – ildjarn May 31 '12 at 02:03
  • Thanks Chet, does that increment the smart pointer's ref count? – Steve H May 31 '12 at 02:03
  • 8
    @SteveH : It doesn't, but why should your function force special object ownership semantics on its callers if it doesn't actually need them? Your function does nothing that wouldn't be better as `void DoSomething(myClass& arg1)`. – ildjarn May 31 '12 at 02:05
  • @SteveH Mine is more of a drive by answer. Unfortunately I don't have time to provide a proper answer explaining the various semantics of passing smart pointers...hence the drive by ;) – Captain Obvlious May 31 '12 at 02:08
  • @ildjarn : I see. I'm just wanting to learn basic rules of thumb about passing things around. Sounds like you would suggest passing only a reference unless ownership is required. – Steve H May 31 '12 at 02:08
  • 2
    @SteveH : The entire purpose of smart pointers is to handle out of the ordinary object ownership issues -- if you don't have those, you shouldn't be using smart pointers in the first place. ;-] As to `shared_ptr<>` specifically, you _must_ pass by value in order to actually share ownership. – ildjarn May 31 '12 at 02:09
  • @ildjarn : wow, I hear from others that raw pointers are to be avoided now. Are there two camps forming? – Steve H May 31 '12 at 02:11
  • @SteveH : No one is advocating raw pointers -- use automatic storage and references, and smart pointers and values otherwise. In your example, a smart pointer is not warranted. – ildjarn May 31 '12 at 02:12
  • 4
    Unrelated: in general, `std::make_shared` is not only more efficient, but also [safer](http://herbsutter.com/gotw/_102/) than the `std::shared_ptr` constructor. – R. Martinho Fernandes May 31 '12 at 03:31
  • Related question: http://stackoverflow.com/questions/327573/c-passing-references-to-boostshared-ptr – Jon Jun 07 '12 at 00:22

5 Answers5

192

I want to pass a shared pointer to a function. Can you help me with that?

Sure, I can help with you that. I assume you have some understanding of ownership semantics in C++. Is that true?

Yeah, I'm reasonably comfortable with the subject.

Good.

Ok, I can only think of two reasons to take a shared_ptr argument:

  1. The function wants to share ownership of the object;
  2. The function does some operation that works specifically on shared_ptrs.

Which one are you interested in?

I'm looking for a general answer, so I'm actually interested in both. I'm curious about what you mean in case #2, though.

Examples of such functions include std::static_pointer_cast, custom comparators, or predicates. For example, if you need to find all unique shared_ptr from a vector, you need such a predicate.

Ah, when the function actually needs to manipulate the smart pointer itself.

Exactly.

In that case, I think we should pass by reference.

Yes. And if it doesn't change the pointer, you want to pass by const reference. There's no need to copy since you don't need to share ownership. That's the other scenario.

Ok, got it. Let's talk about the other scenario.

The one where you share the ownership? Ok. How do you share ownership with shared_ptr?

By copying it.

Then the function will need to make a copy of a shared_ptr, correct?

Obviously. So I pass it by a reference to const and copy to a local variable?

No, that's a pessimization. If it is passed by reference, the function will have no choice but to make the copy manually. If it is passed by value the compiler will pick the best choice between a copy and a move and perform it automatically. So, pass by value.

Good point. I must remember that "Want Speed? Pass by Value." article more often.

Wait, what if the function stores the shared_ptr in a data member, for example? Won't that make a redundant copy?

The function can simply move the shared_ptr argument into its storage. Moving a shared_ptr is cheap because it doesn't change any reference counts.

Ah, good idea.

But I'm thinking of a third scenario: what if you don't want to manipulate the shared_ptr, nor to share ownership?

In that case, shared_ptr is completely irrelevant to the function. If you want to manipulate the pointee, take a pointee, and let the callers pick what ownership semantics they want.

And should I take the pointee by reference or by value?

The usual rules apply. Smart pointers don't change anything.

Pass by value if I'm going to copy, pass by reference if I want to avoid a copy.

Right.

Hmm. I think you forgot yet another scenario. What if I want to share ownership, but only depending on a certain condition?

Ah, an interesting edge case. I don't expect that to happen often. But when it happens you can either pass by value and ignore the copy if you don't need it, or pass by reference and make the copy if you need it.

I risk one redundant copy in the first option, and lose a potential move in the second. Can't I eat the cake and have it too?

If you're in a situation where that really matters, you can provide two overloads, one taking a const lvalue reference, and another taking an rvalue reference. One copies, the other moves. A perfect-forwarding function template is another option.

I think that covers all the possible scenarios. Thank you very much.

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
  • 2
    @Jon: What for? This is all about should I do _A_ or should I do _B_. I think we all know how to pass objects by value/reference, no? – sbi Jun 07 '12 at 10:14
  • 9
    Because prose like "Does that mean I'll pass it by a reference to const to make the copy? No, that's a pessimization" is confusing to a beginner. Passing by a reference to const does not make a copy. – Jon Jun 08 '12 at 12:20
  • 1
    @Martinho I disagree with the rule "If you want to manipulate the pointee, take a pointee,". As stated in [this](http://stackoverflow.com/questions/327573/c-passing-references-to-boostshared-ptr) answer not taking temporary ownership of an object can be dangerous. In my opinion the default should be to pass a copy for temporary ownership to guarantee object validity for the duration of the function call. – radman Mar 09 '13 at 00:16
  • @radman No scenario in the answer you link to applies that rule, so it's completely irrelevant. Please write me a SSCCE where you pass by reference from a `shared_ptr ptr;` (i.e. `void f(T&)`, called with `f(*ptr)`) and the object does not outlives the call. Alternatively write one where you pass by value `void f(T)` and trouble strikes. – R. Martinho Fernandes Mar 09 '13 at 00:44
  • @Martinho check out my [example code](http://pastebin.com/fGRL9def) for the simple self contained correct example. The example is for `void f(T&)` and involves threads. I think my issue is that the tone of your answer discourages passing ownership of shared_ptr's, which is the safest, most intuitive and least complicated way of using them. I would be extremely against ever advising a beginner to extract a reference to data owned by a shared_ptr<> the possibilities for misuse are great and the benefit is minimal. – radman Mar 09 '13 at 04:52
  • The problem in your example is of a different nature: you have threads sharing the object but don't give them shared ownership. Read this answer clearly and you will realize that if you need to share ownership, you pass a shared_ptr. I think my answer encourages not finding *one* whateverst way of doing anything, and instead encourages thinking about what the heck you're trying to do. I don't want beginners (or anyone) to use smart pointers without understanding the idea of ownership. – R. Martinho Fernandes Mar 09 '13 at 15:38
  • @radman Also note how your example passes the reference to `std::ref`, *and the object does indeed outlive the call to `std::ref`*. – R. Martinho Fernandes Mar 09 '13 at 15:44
  • Having one thread pull the rug from under another thread will cause you trouble regardless of what you pass (what if the main() thread mutated the object?). It's completely unrelated to shared_ptr. It's an issue that only comes up when wanting to write multithreaded code without thinking. – R. Martinho Fernandes Mar 09 '13 at 15:51
  • @Martinho It is not unrelated, if the function took a shared_ptr<> then no crash would occur. shared_ptr<> guarantees that the data it holds remains valid regardless of threading concerns. The other major danger is that the function being passed the reference takes a copy and stores it beyond the life of the call, which would cause problems without threading. You are advocating trusting the called function/object with doing the right thing, there are no semantics preventing them. If a full shared_ptr<> is passed then it's on their own head if they want to violate the ownership explicitly. – radman Mar 10 '13 at 03:25
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/25901/discussion-between-r-martinho-fernandes-and-radman) – R. Martinho Fernandes Mar 10 '13 at 03:28
  • What about when you need to pass a derived type as a base type? This requires pointers correct? – daaxix Jun 26 '15 at 18:37
  • nevermind, just read that references avoid the slicing issue. – daaxix Jun 26 '15 at 18:42
  • This answer was difficult to read. Do you have a succinct answer for passing a shared pointer into a function to be stored there for use through out the life of the object? Maybe I should just use standard pointers instead of shared_ptr? Basically I have objects that must refer to each other... – James Nov 05 '16 at 22:59
  • @James "to be stored there for use through out the life of the object" is covered under "Wait, what if the function stores the shared_ptr in a member variable, for example?" Regarding the answer being difficult to read, can you tell me which parts I could improve? FWIW, I wrote this under the assumption that the reader already has some understanding of ownership semantics in C++; I wanted to focus on argument passing specifically, and not muddle it by explaining ownership as well. – R. Martinho Fernandes Mar 20 '17 at 10:48
32

I think people are unnecessarily scared of using raw pointers as function parameters. If the function is not going to store the pointer or otherwise affect its lifetime, a raw pointer works just as well and represents the lowest common denominator. Consider for example how you would pass a unique_ptr into a function that takes a shared_ptr as a parameter, either by value or by const reference?

void DoSomething(myClass * p);

DoSomething(myClass_shared_ptr.get());
DoSomething(myClass_unique_ptr.get());

A raw pointer as a function parameter does not prevent you from using smart pointers in the calling code, where it really matters.

Mark Ransom
  • 299,747
  • 42
  • 398
  • 622
  • 9
    If you're using a pointer, why not just use a reference? `DoSomething(*a_smart_ptr)` – Xeo May 31 '12 at 03:43
  • 6
    @Xeo, you're right - that would be even better. Sometimes you need to allow for the possibility of a NULL pointer though. – Mark Ransom May 31 '12 at 03:47
  • 1
    If you have a convention of using raw pointers as output parameters it's easier to spot in calling code that a variable may change, e.g.: compare `fun(&x)` to `fun(x)`. In the latter example `x` could either be passed by value or as a const ref. It will as stated above also allow you to pass `nullptr`if you're not interested in the output... – Andreas Magnusson Aug 29 '12 at 12:13
  • 2
    @AndreasMagnusson: That pointer-as-output-parameter convention can give a false sense of security when dealing with pointers passed in from another source. Because in that case the call is fun(x) and *x is modified and the source has no &x to warn you. – Zan Lynx Oct 02 '13 at 23:07
  • what if the function call outlives the scope of the caller? There is a possibility that the shared ptr's destructor has been called, and now the function will be trying to access deleted memory, resulting in UB – Sridhar Thiagarajan Oct 21 '19 at 01:50
  • @SridharThiagarajan that's impossible unless the function makes a copy of the pointer and saves it somewhere for reuse later. The calling code scope doesn't change until the called function returns. – Mark Ransom Oct 21 '19 at 02:09
  • it could change if the function is called in a seperate thread, can't it? – Sridhar Thiagarajan Oct 21 '19 at 02:11
  • @SridharThiagarajan there is nothing in the question that implies multi-threading. I don't think `shared_ptr` is guaranteed safe in a multi-threaded environment either, so your problems are just beginning. – Mark Ransom Oct 21 '19 at 02:20
  • I think it's good to note, regardless of whether the OP uses in a multi-threaded context. – Sridhar Thiagarajan Oct 21 '19 at 02:24
5

Yes, the entire idea about a shared_ptr<> is that multiple instances can hold the same raw pointer and the underlying memory will only be freed when there the last instance of shared_ptr<> is destroyed.

I would avoid a pointer to a shared_ptr<> as that defeats the purpose as you are now dealing with raw_pointers again.

R Samuel Klatchko
  • 74,869
  • 16
  • 134
  • 187
2

Passing-by-value in your first example is safe but there is a better idiom. Pass by const reference when possible - I would say yes even when dealing with smart pointers. Your second example is not exactly broken but it's very !???. Silly, not accomplishing anything and defeats part of the point of smart pointers, and going to leave you in a buggy world of pain when you try to dereference and modify things.

djechlin
  • 59,258
  • 35
  • 162
  • 290
  • If you need shared ownership, you _need_ to pass by value, otherwise you're not actually sharing anything. If you don't need shared ownership, then don't pass a `shared_ptr<>` to begin with. – ildjarn May 31 '12 at 02:09
  • @ildjarn - Unsure if I agree with this. Can you produce a case where passing by pointer makes sense and passing by reference doesn't in the first place? – djechlin May 31 '12 at 02:11
  • 3
    No one is advocating passing by pointer, if you mean raw pointer. Passing by reference if there is no ownership contention is certainly idiomatic. The point is, regarding `shared_ptr<>` specifically, that you _must_ make a copy in order to actually share ownership; if you have a reference or pointer to a `shared_ptr<>` then you're not sharing anything, and are subject to the same lifetime issues as a normal reference or pointer. – ildjarn May 31 '12 at 02:13
  • 2
    @ildjarn: Passing a constant reference is fine in this case. The original `shared_ptr` of the caller is guaranteed to outlast the function call, so the function is safe in using the `shared_ptr<> const &`. If it needs to store the pointer for a later time, it will need to copy (at this point a copy must be made, rather than holding a reference), but there is no need to incur the cost of copying *unless* you need to do it. I would go for passing a constant reference in this case.... – David Rodríguez - dribeas May 31 '12 at 02:20
  • 3
    @David : "*Passing a constant reference is fine in this case.*" Not in any sane API -- why would an API mandate a smart pointer type that it's not even taking advantage of rather than just taking a normal const reference? That's almost as bad as lack of const-correctness. If your point is that technically it isn't hurting anything, then I certainly agree, but I don't think it's the Right thing to do. – ildjarn May 31 '12 at 02:21
  • 2
    ... if on the other hand, the function does not need to extend the lifetime of the object, then you can just remove the `shared_ptr` from the interface and pass a plain (`const`) reference to the pointed object. – David Rodríguez - dribeas May 31 '12 at 02:21
  • @ildjarn: Well, I consider it a good guideline, my company considers it a sane guideline (and there are really smart people working here), and heck, even Herb Sutter considers it a good idea [second day's talk in Going Native 2012]. He actually calls passing the `shared_ptr` by value a *pessimization*. Then again, everyone could be wrong :) [C++11 and beyond -- slide 22](http://view.officeapps.live.com/op/view.aspx?src=http%3a%2f%2fecn.channel9.msdn.com%2fevents%2fGoingNative12%2fGN12HerbSutterCpp11VCBeyondDay2.pptx) – David Rodríguez - dribeas May 31 '12 at 02:25
  • 3
    @David : What if your API consumer isn't using `shared_ptr<>` to begin with? Now your API is really just a pain in the neck, as it forces the caller to pointlessly change their object lifetime semantics just to use it. I don't see anything worth advocating in that, other than to say "it isn't technically hurting anything." I don't see anything controversial about 'if you don't need shared ownership, don't use `shared_ptr<>`'. – ildjarn May 31 '12 at 02:26
  • @ildjarn: I think I covered that in a previous comment, if there is no need to extend the lifetime (i.e. no copies of the `shared_ptr` will be taken) then pass a plain reference. But, from your first comment: *If you need shared ownership, you need to pass by value, otherwise you are not actually sharing anything* is false, you need to pass the shared pointer by const-reference and *then* make a copy. Note that the lifetime of the parameter of the function (i.e. the copy in pass-by-value) will never outlast the lifetime of the original `shared_ptr`, so the extra copy is unneeded. – David Rodríguez - dribeas May 31 '12 at 02:32
  • @David : "*I think I covered that in a previous comment*" Agreed, I was (probably needlessly) re-ephmasizing. "*... you need to pass by const-reference to the shared pointer and then make a copy.*" This makes no sense -- how is this semantically any different than passing by value, other than to make it possible for the caller to relinquishing ownership by moving (which is a good thing)? EDIT: misunderstood your last sentence. – ildjarn May 31 '12 at 02:35
  • @ildjarn: let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/11966/discussion-between-david-rodriguez-dribeas-and-ildjarn), but basically you are paying the cost of an atomic increment and gaining nothing out of it. – David Rodríguez - dribeas May 31 '12 at 02:44
  • @DavidRodríguez-dribeas Note that if it is guaranteed that you will need to copy a parameter, pass by value is an optimisation over pass by reference, so that **the copying is part of the public interface.** – curiousguy Jul 24 '12 at 07:30
  • @curiousguy: That is less of an issue for types with *reference* semantics. In particular, there are limited cases where you will want to *copy* (store it in a member?) as the copy and the original *are* the same object. Even then, you would need to take the argument by value and explicitly *move* inside the function (again, the destination of the copy will never be a local variable to the function). – David Rodríguez - dribeas Jul 24 '12 at 14:46
  • @DavidRodríguez-dribeas "_there are limited cases where you will want to copy_" If you do not want to copy, you probably do not need a smart ptr as a parameter. – curiousguy Jul 24 '12 at 16:27
  • @curiousguy: Unless you want to obtain a `weak_ptr`, for example. Discussing corner cases does not really make sense. I already mentioned before that changing the signature to take a reference instead of a smart pointer might be the way to go... – David Rodríguez - dribeas Jul 24 '12 at 16:35
  • Can we move this discussion to here: http://chat.stackoverflow.com/rooms/11966/discussion-between-david-rodriguez-dribeas-and-ildjarn – Kev Jul 24 '12 at 23:46
1

in your function DoSomething you are changing a data member of an instance of class myClass so what you are modifying is the managed (raw pointer) object not the (shared_ptr) object. Which means at the return point of this function all shared pointers to the managed raw pointer will see their data member: myClass::someField changed to a different value.

in this case, you are passing an object to a function with the guarantee that you are not modifying it (talking about shared_ptr not the owned object).

The idiom to express this is through: a const ref, like so

void DoSomething(const std::shared_ptr<myClass>& arg)

Likewise you are assuring the user of your function, that you are not adding another owner to the list of owners of the raw pointer. However, you kept the possibility to modify the underlying object pointed to by the raw pointer.

CAVEAT: Which means, if by some means someone calls shared_ptr::reset before you call your function, and at that moment it is the last shared_ptr owning the raw_ptr, then your object will be destroyed, and your function will manipulate a dangling Pointer to destroyed object. VERY DANGEROUS!!!

organicoman
  • 154
  • 6