Is shared_ptr dangerous?

Just a quick question to anyone who has used shared_ptr before. I just tried using TR1's implementation of shared_ptr (I don't know if this applies to boost), but to my frustration, I found no Detach function, to get the pointer without destroying the pointer.

I also found this resource when googling for it:
http://www.archivum.info/comp.std.c+.../msg00043.html

It specifically says shared_ptr lacks detach and is dangerous:

Quote:

---

I think this simulates new/delete semantics with a dangling pointer
check. Yes, it is not pretty, and shared_ptr does not have a detach().

But worse, it does not work (just like plain new/delete does not) in
the case of two objects A and B, where, if one is destroyed, it should
destroy the other with it. If both in their destructors call destroy,
and one of the objects gets destroyed from the outside, infinite
recursion occurs: A dtor runs -> destroys reference to B -> B dtor runs
-> destroys reference to A -> ...

---

Is there any truth to this claim? It seems like, if true, C++ truly lacks a good smart pointer in this case and it should be best to stick to custom ones or perhaps the one in boost is better?

Yea, if you have a circular reference and circumvent reference logic by manual deletion you get infinite recursion if you don't protect against it. Of course, a couple of questions should immediately arise: Why have you ignored the suggestion to use 'weak_ptr' to protect against circular references? Why have you bypassed reference logic?

Soma

I don't know what weak_ptr is or how to use it. I haven't used any smart pointers but my own up until now and it has worked great in all scenarios.

There are some rules that apply to reference-counted pointers like shared_ptr, or also your own CMemoryManager:

1) You can't have a Detach(), release(), or whatever you want to call it. It's either unsafe or unreliable. Put simply: if there's more than one reference, calling release() can either fail, or it can actually detach the memory from the reference count, thus modifying pointers you don't really know about - and the owners of these pointers won't be happy.
There is no use case for release() where failing is acceptable.

2) Beware the circular reference. If A has a shared_ptr to B and B has a shared_ptr to A (or A to B, B to C, C to A, or anything like it), the two objects keep each other alive and you have a leak. If you have cycles (which, except for some very complex systems, are inevitably design problems), you have to break them manually. You don't do this by deleting one of the objects explicitly. You never delete anything held by a shared_ptr explicitly. Absolutely never. What you do is, you call shared_ptr::reset() or its equivalent. Now, reset() sets the pointer to NULL. It decrements the reference count. If the count is zero, it deletes the object. In that order! (Obviously it has to store the pointer in some temporary. But its external state must say "I'm NULL" before it deletes the other object.) Otherwise, you get the recursion issue mentioned here.
Better than breaking cycles is avoiding them. A weak pointer is a companion to a reference-counted (strong) pointer. The weak pointer points to the object, but does not increment the reference count. Its existence does not keep the object alive. However, the weak pointer must be able to recognize that the referenced object has been deleted.
To use a weak pointer, you have to "fix" it. You do this by creating a temporary (local) strong pointer from the weak pointer. If the weak pointer's pointee no longer exists, you'll get an error. If it does, you get the strong pointer. This ensures that the object cannot be deleted from under your feet while you're using it.

Quote:

---

Originally Posted by CornedBee

1) You can't have a Detach(), release(), or whatever you want to call it. It's either unsafe or unreliable. Put simply: if there's more than one reference, calling release() can either fail, or it can actually detach the memory from the reference count, thus modifying pointers you don't really know about - and the owners of these pointers won't be happy.
There is no use case for release() where failing is acceptable.

---

I do not see the harm if used properly. If detaching the pointer, and if the reference count is one, then you are responsible for deleting it. But you can simple attach it to a memory manager again and let it destroy if this is the case (can work like RAII to delete it when the function ends, although auto_ptr might perhaps be more suitable for that).
If the reference count is not 1, then I must never call delete on the pointer. I can use the pointer as I wish, and modifying its memory, but I cannot delete the object, since that would make the smart pointer attempt to free already freed memory at some point.

What I see as dangerous are these weak_ptrs. They can be abused and cause all sorts of havoc, more so than Detach, or equally much.
I think I'll stick to my own smart pointer.

Quote:

---

Originally Posted by Elysia

I do not see the harm if used properly. If detaching the pointer, and if the reference count is one, then you are responsible for deleting it.

---

The reference count being one is the non-problematic case.

Quote:

---

If the reference count is not 1, then I must never call delete on the pointer. I can use the pointer as I wish, and modifying its memory, but I cannot delete the object, since that would make the smart pointer attempt to free already freed memory at some point.

---

So basically, detaching when the reference count is not 1 does what? Give you the raw pointer? Perhaps also reset the smart pointer? What use is that?

I would be very interested in seeing a good use case for your detach() semantics. I claim that there is none - that any use case you may have results from not distinguishing between various types of smart pointers (scoped_ptr, shared_ptr, move_ptr).

Quote:

---

What I see as dangerous are these weak_ptrs. They can be abused and cause all sorts of havoc, more so than Detach, or equally much.

---

Do you have specific scenarios in mind, or is that a gut reaction?

Quote:

---

Originally Posted by CornedBee

So basically, detaching when the reference count is not 1 does what? Give you the raw pointer? Perhaps also reset the smart pointer? What use is that?

I would be very interested in seeing a good use case for your detach() semantics. I claim that there is none - that any use case you may have results from not distinguishing between various types of smart pointers (scoped_ptr, shared_ptr, move_ptr).

---

Maybe there is, maybe there isn't, but mostly I wanted to avoid duplicate code. It's not a biggie, but depending on the path in the code, I needed the pointer and sometimes not, because one path would be to pass the pointer to a thread and one just a call to one function.

Current code looks something like:

Code:

---

                if (!bThread)
                {
                        if ( FindFilesInternal(pArgs, pBuffer, nCurrent, nTotal) )
                                return pBuffer;
                        else
                                return PPNULL;
                }
                else
                {
                        pArgs->pFiles = pBuffer;
                        ASSERT(pBuffer != PPNULL);
                        NewThreadTF(NULL, pArgs.PassByThread(), &ThreadEntryA);
                        return PPNULL;
                }

---

In the case of the thread-path, it would have to detach the pointer and pass it to the thread.

So, not a huge deal. I was mostly investigating.

Quote:

---

Do you have specific scenarios in mind, or is that a gut reaction?

---

Granted, I haven't used shared_ptr and weak_ptr enough to give a real sample, but it's a reaction. Since weak_ptr won't increase the ref count, the object can be deleted prematurely, just as you can delete a pointer that you detach.

Quote:

---

Originally Posted by Elysia

Since weak_ptr won't increase the ref count, the object can be deleted prematurely, just as you can delete a pointer that you detach.

---

No. The whole point of weak_ptr is that it is used in situations where
a) the owner is dead by the time the pointee is deleted or
b) the owner can work with or without the pointee - it's just that if there is a pointee, something must be done.

Consider, for example, event subscription. If all subscribers must be held by a shared_ptr in the first place, then the subscription service can hold weak_ptrs to them. This way, it doesn't block destruction of the subscribers, and since a weak_ptr can detect destruction, the service simply kicks destroyed obejcts from its list.


I fail to see the point in your example.

Obviously there is a lot to think and learn about weak_ptr to use them properly. Failure to do so can cause problems in the code.
But I think the same could hold true to detach? You simply detach the pointer, and let the pointer go away along with the smart pointer.
But this wouldn't really be a safe method for passing the object to a thread unless the ref count is 1 or the object is not being used on multiple threads. I'm not sure what the optimal solution would be with shared_ptr?

detach() is inherently bad because its behaviour is weird. That's all there is to it.

weak_ptr's behaviour, on the other hand, is quite logical, if not exactly trivial.

To pass a shared_ptr to a different thread, you create the new thread, passing the address of the shared_ptr, and wait for the new thread to have created its own copy before continuing execution. Incidently, if you use Boost.Thread and give the thread function a value argument of shared_ptr type, all that stuff is handled automatically. In other words, safely passing a shared_ptr to a new thread is as simple as this:

Code:

---

void threadfunc(shared_ptr<widget> w)
{
  // do stuff
}

int main()
{
  shared_ptr<widget> p(new widget);
  thread t(bind(threadfunc, p));
  // do stuff
}

---

Boost.Thread does sound very useful, and I'm thinking it might just work similar to my own thread function I've made back in the days. Perhaps better.

Wait, did I read that correctly?
shared_ptr doesn't let you get at its underlying pointer?
If that's true, how could you pass that pointer held by a shared_ptr to an older function that knows nothing about shared_ptr's such as all the C & Windows API functions?

Sure it does. Use get().

But that doesn't affect the reference count or the shared_ptr's internal pointer in any way. And make sure that the shared_ptr you got the raw pointer from lives at least as long as the raw pointer, or you might get a dangling pointer.

Quote:

---

Originally Posted by CornedBee

Sure it does. Use get().

But that doesn't affect the reference count or the shared_ptr's internal pointer in any way. And make sure that the shared_ptr you got the raw pointer from lives at least as long as the raw pointer, or you might get a dangling pointer.

---

OK good. You two were starting to scare me for a second. :)
Then what's all this talk about a detach() function? How does it differ from get()?

Quote:

---

Originally Posted by Elysia

I just tried using TR1's implementation of shared_ptr (I don't know if this applies to boost), but to my frustration, I found no Detach function, to get the pointer without destroying the pointer.

---

The commonly accepted definition of detach() is that it is a function that makes the smart pointer stop managing the memory it points to. In other words, it means, "give me the raw pointer and set yourself to NULL".
In std::auto_ptr, this function is called release().

If the above is not what Elysia meant by Detach(), we've wasted a lot of typing ...

Quote:

---

Originally Posted by CornedBee

The commonly accepted definition of detach() is that it is a function that makes the smart pointer stop managing the memory it points to. In other words, it means, "give me the raw pointer and set yourself to NULL".
In std::auto_ptr, this function is called release().

If the above is not what Elysia meant by Detach(), we've wasted a lot of typing ...

---

OK thanks, I'm familiar with release(), but I never saw a detach() before.
I can certainly see the danger of having a release() or detach() function in a shared_ptr.

Quote:

---

Originally Posted by CornedBee

The commonly accepted definition of detach() is that it is a function that makes the smart pointer stop managing the memory it points to. In other words, it means, "give me the raw pointer and set yourself to NULL".
In std::auto_ptr, this function is called release().

If the above is not what Elysia meant by Detach(), we've wasted a lot of typing ...

---

Yes, that's what it means. I wanted to release the smart pointer its duty to manage to memory and give it to be under a certain circumstance. ATL's CComPtr, for example, supports this.

O_o

Does 'CComPtr::Detach' alter the reference? Or does it just return the pointer and null the member for that particular instance of 'CComPtr'?

Can you give more information about this "certain circumstance"?

Soma

Quote:

---

Releases ownership of a pointer, sets the CComPtrBase::p data member variable to NULL, and returns a copy of the pointer.

---

That's why CComPtr::Detach does. I can't remember more details right now. It's just from the documentation.
But it should guarantee that it returns the pointer and does not Release it beforehand (which shared_ptr would do).

CComPtr has the advantage that the reference counting of COM objects is internal and publicly documented. If a shared_ptr gives up its pointer, it has to decrement the refcount, because there's no way for the user to do it.