Resource Pointer Design

Started by
5 comments, last by vreality 11 years, 9 months ago

Working with the D3D API involves handling API resources via pointers. The API user is responsible for disposing of those resources.

In the spirit of modern C++ style, this is a job for some sort of smart pointer. But unique_ptr isn't a great fit, for a couple of reasons.

The way resources are acquired
The D3D API produces resources via "out parameters", which would require explict temporary pointers to transport resource addresses to owning unique_ptrs:


resource_type * pResource;     // Temporary pointer.
D3D_Make_Resource(&pResource); // D3D style of producing resources.
upResource.reset(pResource);   // unique_ptr's style of assigning.

 

The way resources are freed
D3D resources are released, not deleted. Not only would unique_ptr require a custom deleter type to free the resources, it would require a unique deleter type per resource type, because each resource class has its own member Release() method, rather than sharing a common non-member function, like delete():


unique_ptr<resource_type, resource_releaser<resource_type>> upResource;

 

These aren't exactly show stoppers, but they're enough to take a couple of minutes to see if I can't do a little better. So I threw together a resource pointer class along the lines of this:


template<typename T> class d3d_resource_ptr
{
  public:
    d3d_resource_ptr() : pResource(nullptr) {}
    ~d3d_resource_ptr() {Release();}

    T & operator * () const {return *pResource;} // Dereferencing works
    T * operator -> () const {return pResource;} // like a normal pointer.

  private:
    d3d_resource_ptr (d3d_resource_ptr const &); // Unique ownership. No need
    d3d_resource_ptr & operator = (d3d_resource_ptr const &); // for copying (or moving).

    void Release() {if(pResource != nullptr){pResource->Release();} pResource = nullptr;}

    T * pResource;
};

The question is how best to get and set the raw pointer. Which is the lesser evil?

We could use a reference operator and implicit pointer conversion:


public:
  T * * operator & () {Release(); return &pResource;} // Pointer to pointer, for reassignment.

  operator T * () const {return pResource;} // Conversion to T pointer, for storage or passing.

Which provides about as much convenience as possible:


D3D_Make_Resource(&rpResource); // operator & passes a T * * so the API can assign through it.
D3D_Use_Resource(rpPesource);   // operator T * generates a T * so the API can use it.

The evil here is that the resource pointer interface is a little too clever. It's hiding the fact that taking a reference automatically releases any resource (in anticipation of assigning a new one). That's an accident waiting to happen. And I suspect that implicit pointer conversions are trouble makers as well.

Alternatively, we could use explicit functions:


public:
  T * Get() const {return pResource;}
  T * * Assign() {Release(); return &pResource;}

private:
  d3d_resource_ptr * operator & (); // Protect against errant references.

Which produces code like this:


D3D_Make_Resource(rpResource.Assign());
D3D_Use_Resource(rpResource.Get());

I guess the thing that bugs me the most about this is that I can't think of a better name for Assign(). I don't think its intention is entirely clear. What is clear is that this approach is not as clean as the previous option, and adds noise to already noisy function calls which handle several resources at a time.

So which is the lesser of the two evils? Or is there another way which is even less evil?

Would this be better or worse? :


public:
  void Release() {if(pResource != nullptr){pResource->Release();} pResource = nullptr;} // Now public.
  T * * operator & () {assert(pResource == nullptr); return &pResource;} // Require Release() rather than calling it.

 

Advertisement
One option is to use an existing, well-known COM smart pointer like CComPtr. The fact that the interface is well-known helps compensate for some of the oddities that you've noticed trying to design your own smart pointer.
It's been many, many years since I've worked with C++ and DirectX, but I remember that shared_ptr had the ability for you to tie in specialized destructor behavior.

Which produces code like this:
D3D_Make_Resource(rpResource.Assign());
D3D_Use_Resource(rpResource.Get());


I guess the thing that bugs me the most about this is that I can't think of a better name for Assign(). I don't think its intention is entirely clear. What is clear is that this approach is not as clean as the previous option, and adds noise to already noisy function calls which handle several resources at a time.

So which is the lesser of the two evils? Or is there another way which is even less evil?

Would this be better or worse? :
public:
void Release() {if(pResource != nullptr){pResource->Release();} pResource = nullptr;} // Now public.
T * * operator & () {assert(pResource == nullptr); return &pResource;} // Require Release() rather than calling it.


IMHO, both are worst.
You should never get the raw pointer's address from a smart pointer. That is not the way smart pointer is supposed to work.
That will cause the raw pointer being assigned without notifying the smart pointer. That may either cause memory leak or bad pointer.

What you'd better do is,
SomeType * pointer = NULL;
D3D_make_resource(&pointer);
SomeSmartPointer.reset(pointer);

Also, you'd better prefer an existing smart pointer rather than rolling your own one. It may be not as easy as you thought.

https://www.kbasm.com -- My personal website

https://github.com/wqking/eventpp  eventpp -- C++ library for event dispatcher and callback list

https://github.com/cpgf/cpgf  cpgf library -- free C++ open source library for reflection, serialization, script binding, callbacks, and meta data for OpenGL Box2D, SFML and Irrlicht.

On 7/12/2012 at 10:41 PM, wqking said:

You should never get the raw pointer's address from a smart pointer. That is not the way smart pointer is supposed to work.
That will cause the raw pointer being assigned without notifying the smart pointer. That may either cause memory leak or bad pointer.

Smart pointers manage ownership, and resource lifetime. While it's certainly possible to misuse raw pointers, there are safe ways to use them which are preferable to reference counted shared ownership (when it's unnecessary or inappropriate). Hence std::unique_ptr and std::unique_ptr::get().

In this case, the D3D API only accepts raw pointers. So if we can't get them, we're dead in the water. But don't worry, D3D won't store them and try to use them later.

On 7/12/2012 at 10:41 PM, wqking said:

Also, you'd better prefer an existing smart pointer rather than rolling your own one. It may be not as easy as you thought.

If a general purpose smart pointer would do, then you're right - std::unique_ptr and std::shared_ptr are top notch. std::unique pointer is my top choice for this ownership scheme.

It turns out that these resources don't quite fit the mold. You can't delete them, and they each have unique Release members. But the fact that this is such a narrowly focused problem greatly reduces the complexity of arriving at an acceptable design.

As SiCrane pointed out, there are already COM smart pointers, which are created for this very purpose. Is there something that will prevent you from using them?
On 7/12/2012 at 5:34 PM, SiCrane said:

One option is to use an existing, well-known COM smart pointer like CComPtr. The fact that the interface is well-known helps compensate for some of the oddities that you've noticed trying to design your own smart pointer.

Ah...


template <class T> class CComPtrBase
{
  ...
  public:
    //The assert on operator& usually indicates a bug. If this is really
    //what is needed, however, take the address of the p member explicitly.
    T** operator&() throw()
    {
      ATLASSERT(p==NULL);
      return &p;
    }
    ...
    T* p;
    ...

They're making the raw pointer public and going with the assert in operator &.

This topic is closed to new replies.

Advertisement