Is it advisable to use a scoped_ptr in CreateFrame? (D3DX9)

Started by
3 comments, last by BitMaster 8 years, 10 months ago

This code snippet always throws exceptions which has something to do with the heap. When the breakpoint is hit, pFrame is a null pointer. I want the pFrame to be scoped because I want the pFrame to manage itself when it goes out of scope of CreateFrame, I am not sure because the ownership probably has to be transferred to ppNewFrame so that a shared_ptr may be needed.

Actually, I have tested both of the scenarios, using shared and scoped, both didn't work

Any suggestions?

Thanks Jack


HRESULT CAllocateHierarchy::CreateFrame( LPCSTR Name, LPD3DXFRAME* ppNewFrame )
{
    HRESULT hr = S_OK;
    *ppNewFrame = NULL; 
    boost::scoped_ptr pFrame(new D3DXFRAME_DERIVED);	 

    //Copy name
    if (Name != NULL)
    {
	hr = AllocateName(Name, &pFrame->Name);
    }

    //Set the transformation matrices
    D3DXMatrixIdentity(&pFrame->TransformationMatrix);
    D3DXMatrixIdentity(&pFrame->CombinedTransformationMatrix);

    pFrame->pMeshContainer = NULL; 
    pFrame->pFrameSibling = NULL;
    pFrame->pFrameFirstChild = NULL;

    //Return the new bone...
    *ppNewFrame = (D3DXFRAME*)pFrame.get();

    return hr;
}
Advertisement

There's a built-in contradiction in your post: you say you want the pointer to be scoped to the CreateFrame function (meaning it will be freed upon leaving the function) but you also want to return it for use outside of the function (meaning it can't be freed upon leaving the function). Which is it? And I think just returning a pointer like that does suggest that you have a structural problem with your memory ownership model. Who (meaning which program module/component) owns this pointer, who eventually frees it, and why does CreateFrame have to allocate it, if it's just doing some minor initialization work? You need to think about these things, just trying out every smart pointer variety until it works doesn't work very well smile.png

“If I understand the standard right it is legal and safe to do this but the resulting value could be anything.”

Hello, I actually want to use that pointer outside of CreateFrame, I must misunderstand the usage of scoped_ptr's

Yes, Other modules like the renderers will use that frame again.

When I do this

D3DXLoadMeshHierarchyFromX(..., &mRoot,...);

The D3DXFRAME mRoot should be getting a shared_ptr instead, but it still throws exceptions at me when I do so...

probably this object in the CMesh class has to be a shared_ptr as well.

boost::shared_ptr<D3DXFRAME> mRoot;

But

D3DXLoadMeshHierarchyFromX doesn't allow me to load &mRoot

as parameter.

Thanks

Jack

If you still have a single, unique owner at a time (which you should verify), then std::unique_ptr would be the way to go -- you can move it around, thus transferring ownership.

(In contrast, boost::scoped_ptr cannot be moved, let alone copied.)

http://stackoverflow.com/questions/11496826/transferring-ownership-to-function-with-stdunique-ptr

http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/

https://msdn.microsoft.com/en-us/library/hh279676.aspx

It's also available in Boost:

http://www.boost.org/doc/libs/master/doc/html/move/reference.html#header.boost.move.unique_ptr_hpp

http://www.boost.org/doc/libs/master/doc/html/boost/movelib/unique_ptr.html

TL;DR: you have to analyze and understand the exact ownership semantics that apply to your case at hand, everything else should follow from that.

In particular, you may want to familiarize yourself (thoroughly) with GotW #89 to #91:

http://herbsutter.com/2013/05/29/gotw-89-solution-smart-pointers/

http://herbsutter.com/2013/05/30/gotw-90-solution-factories/

http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/

When I do this
D3DXLoadMeshHierarchyFromX(..., &mRoot,...);
The D3DXFRAME mRoot should be getting a shared_ptr instead, but it still throws exceptions at me when I do so...

probably this object in the CMesh class has to be a shared_ptr as well.
boost::shared_ptr<D3DXFRAME> mRoot;


You seem to have some fundamental misconceptions here. A managed pointer (whether is is boost::scopred_ptr, std::shared_ptr, std::unique_ptr or any other) is not change-type-and-forget replacement. It requires significant engineering to get right. According to the documentation D3DXLoadMeshHierarchyFromX wants a pointer to a pointer to an D3DXFRAME. Not anything else, certainly not a pointer to an boost::shared_ptr<D3DXFRAME> which is (despite similar semantics) a completely different beast (and if you think about casting the error away, that will just blow up; badly).
What you need to do is something like that:



D3DXFRAME* tempRoot = nullptr;
HRESULT result = D3DXLoadMeshHierarchyFromX(..., &tempRoot,...);
if (result is a success)
{
   mRoot.reset(tempRoot, [] (D3DXFRAME* p) { /* release p in the correct way */ });
   // ...
}
else
{
   // handle error, clean up tempRoot if it was allocated even with the error...
}
Also note that you cannot just pass the D3DXFRAME* into a shared_ptr (or any other general smart pointer) because as far as I know you cannot correctly free that with a 'delete'. That means boost::scoped_ptr is out, although std::unique_ptr can have a custom deleter. Note that you can forgo the added deleter in the mRoot.reset(...) call if you correctly specialize std::default_delete.

However, these technical details aside, the important points of this thread are that you really need to think about things like ownership. You cannot just throw smart pointers at the problem in the hope it will go away.

This topic is closed to new replies.

Advertisement