5

I'm on Ubuntu 14.04 using GCC 4.8.4 and I have code similar to the following:

std::shared_ptr<MyClass> my_shared_object = set elsewhere...
MyFunction(*my_shared_object);

Where MyFunction's signature looks like this:

void MyFunction(const MyClass& my_object)

The full code can be found here

However, I am finding that my_object actually goes out of scope within the context of MyFunction. My thought was that the my_shared_object will only release its contents once it goes out of scope, meaning after the MyFunction has returned. I am not sure if I am either misunderstanding std::shared_ptr or if maybe this is a GCC bug.

I guess the question boils down to: when I dereference a std::shared_ptr, does that guarantee that the std::shared_ptr will persist as long as the dereference is being used?

LogicStuff
  • 19,397
  • 6
  • 54
  • 74
kip622
  • 399
  • 5
  • 16
  • 1
    Dereferencing a `std::shared_ptr` will result in a naked pointer, and there's no way for the `std::shared_ptr` to keep track of what happens to further copies of the naked pointer. If you want the object to persist during the scope of your function, then you should pass the `std::shared_ptr` by value. Copying the `std::shared_ptr` signals the intended behaviour of shared ownership. – Felix Glas Nov 19 '15 at 19:42
  • 2
    The shared pointer is valid before and after the function call (hence, the reference, too), unless some stupid elsewhere... happens. –  Nov 19 '15 at 19:46
  • @Snps I agree that your solution would fix the problem. However, I also feel like @Dieter's answer _should_ be correct as well -- ``my_shared_object`` does not go out of context until after ``MyFunction`` returns! So I am thinking maybe the compiler is somehow over-optimizing it – kip622 Nov 19 '15 at 19:48
  • @DieterLücking Yes, that seems correct. But as the question stands, it seems as OP wants to know whether `std::shared_ptr` can keep track of references to its internal object. – Felix Glas Nov 19 '15 at 19:49
  • How are you determining that `*my_shared_object` has gone out of scope? – Andre Kostur Nov 19 '15 at 23:07

3 Answers3

7

Whatever is managed by a std::shared_ptr will be destroyed the moment there's no std::shared_ptr left asserting a claim, all other ways to refer to it are irrelevant.

And local variables are only destroyed upon leaving the respective scope. Dereferencing a std::shared_ptr does not modify it in any way.

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
  • So do you think it is likely that this is some GCC optimization that is prematurely freeing the shared_ptr and this is why the dereferenced object goes out of context in MyFunction? – kip622 Nov 19 '15 at 19:39
  • 1
    In your question here, there's nothing which would allow the `std::shared_ptr` to be modified before that function returns, and I doubt gcc will that catastrophically mis-optimize it. The problem is probably somewhere else. – Deduplicator Nov 19 '15 at 19:41
  • 1
    @kip622 Since you pass a `const` reference how is possible that the object goes out of scope in your function? You don't even pass the object to it but a `const` qualified alias of it. Somewhere else you're screwing things up. – 101010 Nov 19 '15 at 19:42
  • @Deduplicator thanks for looking at the full code.. I know it's tough to follow without knowing the context of the pipeline it's in. But it is clear to see that the shared_ptr should not go out of context until much after the function call... yet it does! – kip622 Nov 19 '15 at 19:51
  • What is `FeatureCorrespondence`? Does any of them contain pointers to anything passed into `MatchImagePair` after the call? If so, is the `keypoints_and_descriptors_cache_` guaranteed to hold on to the fetched data until that's gone too? The last seems unlikely, which could explain your bug. – Deduplicator Nov 19 '15 at 19:57
  • @Deduplicator FeatureCorrespondence can be found [here](https://github.com/sweeneychris/TheiaSfM/blob/master/src/theia/matching/feature_correspondence.h#L48) (no pointers). In `MatchImagePair`, there is one object that attempts to get the address of a field in `KeypointsAndDescriptors` [see here](https://github.com/sweeneychris/TheiaSfM/blob/master/src/theia/matching/cascade_hashing_feature_matcher.cc#L164). That line seems to be the problem. The value type of the cache is a `std::shared_ptr`, so it should not be able to evict the data until the `shared_ptr` no longer exists – kip622 Nov 19 '15 at 20:44
  • @kip622: Yep, that's the line leaking a raw pointer. Making the question whether your cache holds on to everything... – Deduplicator Nov 19 '15 at 20:58
  • @Deduplicator [this line](https://github.com/sweeneychris/TheiaSfM/blob/master/src/theia/util/lru_cache.h#L147) is the one evicting the entry in the cache. Since `ValueType` in this case is a `std::shared_ptr` I believe that erase command cannot complete until the shared_ptr can be freed. This means that when `ValueType` is a `std::shared_ptr` cannot evict an entry until it is sure that the shared_ptr is not being used elsewhere, right? – kip622 Nov 19 '15 at 22:04
  • The point of shared_ptr's is that the managed object is killed the moment none point at it anymore. And that's what `.erase()` does. So you found the second part of your bug. (You might want to prefer evicting elements *only* in the cache over those also held elsewhere, but that's something else.) – Deduplicator Nov 19 '15 at 22:14
  • @Deduplicator thanks so much! But for clarification, there should be pointers existing in the cache AND locally in the feature_matcher.h file (notice that the Fetch method in the cache returns ValueType aka shared_ptr). So the shared_ptr should not be killed until it is killed in feature_matcher.h if I'm understanding correctly... – kip622 Nov 19 '15 at 22:21
  • @kip622: Every iteration of the loop replaces the variables declared in the loop though. So it happens at the end of each iteration. – Deduplicator Nov 19 '15 at 22:25
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/95624/discussion-between-kip622-and-deduplicator). – kip622 Nov 19 '15 at 22:25
4

std::shared_ptr doesn't know about your reference (or your reference doesn't know about the std::shared_ptr) to the object being held, it's aware only of other std::shared_ptrs sharing the ownership of the same object. Therefore, as soon as the last std::shared_ptr goes out of scope, the object gets destructed and you end up with a dangling reference.

However, that should not be the case here and your std::shared_ptr should get properly destructed after the program flow leaves its scope, which I don't understand how, could happen before the call to MyFunction.

LogicStuff
  • 19,397
  • 6
  • 54
  • 74
  • Same question as posed to Deduplicator: So do you think it is likely that this is some GCC optimization that is prematurely freeing the shared_ptr and this is why the dereferenced object goes out of context in MyFunction? – kip622 Nov 19 '15 at 19:39
  • That's really weird. You should provide more concise MCVE. Well, what you linked is not MCVE at all. – LogicStuff Nov 19 '15 at 19:43
1

std::shared_ptr's maintain an internal reference count. This count corresponds to number of objects sharing the object. Everytime a shared_ptr goes out of scope, the internal reference count is decremented. when the internal reference count falls to zero, the memory is de-allocated.

Nandu
  • 808
  • 7
  • 10
  • Thanks for the explanation. I think I understand the basics of `shared_ptr`. The problem here is that it seems to be freed before the exiting the full context, as the question describes – kip622 Nov 19 '15 at 19:41
  • you need to pass the my_shared_object either by value or reference so that the ref count gets incremented within MyFunction – Nandu Nov 19 '15 at 19:42
  • Being pedantic: It's more "sharing *ownership of* the object". And the memory can only be freed the moment the last `std::shared_ptr` relinquishes its claim if there are no `std::weak_ptr`s or the management-object and the managed object were not allocated at once (like a good `std::make_shared` would do). – Deduplicator Nov 19 '15 at 19:44
  • yes. In his code sample, he is not sharing within MyFunction. void MyFunction(const std::shared_ptr& my_object) (or) MyFunction(const std::shared_ptr my_object), myFunction(my_shared_object) would fix the problem – Nandu Nov 19 '15 at 19:46
  • by passing MyFunction(*my_shared_object); he is de-referencing the managed pointer directly and not sharing – Nandu Nov 19 '15 at 19:48
  • 1
    No, not sharing ownership works because the caller retains ownership, and cannot relinquish it before the callee returns. – Deduplicator Nov 19 '15 at 20:03