Member objects, references and shared_ptr

Question

So, I've found myself doing this a lot, and wonder if it's correct practice (this probably won't compile - I'm writing this on my phone):

class Shared { private: int _x; public: void X(int newValue) { _x = newValue; } int X() { return _x; } Shared(void) : _x(0) { } }; class Owner { private: shared_ptr<Shared> _shared; public: const Shared& Shared() const { return *_shared; } void Shared(const Shared& newValue) { _shared.reset(&newValue); } void DoSomethingWithShared() { /// yeah, this could be cleaner, not the point! _shared.X(_shared.X() + 1); } }; void CreateStuff(Owner& a, Owner &b) { Shared s; a.Shared(s); b.Shared(s); } int main(int argc, char *argv[]) { Owner a; Owner b; CreateStuff(a,b); a.DoSomethingWithShared(); b.DoSomethingWithShared(); ///... /// "Shared" instance created in CreateStuff() hopefully lives until here... }

The idea is that multiple instances of Owner need a shared resource of type Shared.

Is CreateStuff() an error? (ie, does s go out of scope, leaving a and b with invalid pointers to a destroyed object? (Am I returning the address of a temporary in a roundabout way?)
Are there any other scope/GC issues I'm not seeing?
Is there an easier way to do this?

Note that this is a pure C++ question. Nothing C++11 or VS specific. Changed the tags accordingly. — Eran
– Eran, Commented Dec 20, 2013 at 21:34

Eran · Accepted Answer · 2013-12-20 21:29:43Z

CreateStuff is definitively wrong. You're (eventually) passing a pointer to a local variable into the shared_ptrs, which outlive that variable. Once it goes out of scope, you'll have two dangling pointers inside those _shareds.

Since you're using smart pointers, why not dynamically allocate that Shared on the heap, and let the smart pointers worry about deleting it when they're done?

void CreateStuff(Owner& a, Owner &b) { std::shared_ptr<Shared> s(new Shared); a.Shared(s); // have that Shared() modified to take the shared_ptr, b.Shared(s); // of course }

I'm assuming that, in the ::Shared() setter, calling shared_ptr::reset() makes them belong to the same ownership group. Is that correct?
Oh, that's a point I actually missed - when passing the raw pointer, the two shared_ptrs don't know about each other. As soon as one goes out of scope, it will try to delete the object. Had it been possible, it would have left the other with a dangling pointer. Changing the arguments to a shared_ptr, as I've already suggested, will fix this as well.
void Shared(std::shared_ptr<Shared> newValue) { _shared = std::move(newValue); } It's best to take newValue by value, since it will also implicitly accept std::unique_ptr<Shared>.

Collectives™ on Stack Overflow

Member objects, references and shared_ptr

1 Answer 1

3 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Related