views:

132

answers:

4

Hi,

I've been trying to use smart pointers to upgrade an existing app, and I'm trying to overcome a puzzle. In my app I have a cache of objects, for example lets call them books. Now this cache of books are requested by ID and if they're in the cache they are returned, if not the object is requested from an external system (slow operation) and added to the cache. Once in the cache many windows can be opened in the app, each of these windows can take a reference to the book. In the previous version of the app the programmer had to maintain AddRef and Release, when every window using the Book object was closed, the final Release (on the cache manager) would remove the object from the cache and delete the object.

You may have spotted the weak link in the chain here, it is of course the programmer remembering to call AddRef and Release. Now I have moved to smart pointers (boost::intrusive) I no longer have to worry about calling AddRef and Release. However this leads to a problem, the cache has a reference to the object, so when the final window is closed, the cache is not notified that no-one else is holding a reference.

My first thoughts were to periodically walk the cache and purge objects with a reference count of one. I didn't like this idea, as it was an Order N operation and didn't feel right. I have come up with a callback system, which is better but not fantastic. I have included the code for the callback system, however I was wondering if anyone had a better way of doing this?

class IContainer
{
public:
    virtual void FinalReference(BaseObject *in_obj)=0;
};

class BaseObject 
{
    unsigned int m_ref;

public:
    IContainer *m_container;

    BaseObject() : m_ref(0),m_container(0)
    {
    }

    void AddRef()
    {
        ++m_ref;
    }
    void Release()
    {
        // if we only have one reference left and we have a container
        if( 2 == m_ref && 0 != m_container )
        {
            m_container->FinalReference(this);
        }

        if( 0 == (--m_ref) )
        {
            delete this;
        }
    }
};

class Book : public BaseObject
{
    char *m_name;
public:
    Book()
    {
        m_name = new char[30];
        sprintf_s(m_name,30,"%07d",rand());
    }
    ~Book()
    {
        cout << "Deleting book : " << m_name;
        delete [] m_name;
    }

    const char *Name()
    {
        return m_name;
    }
};

class BookList : public IContainer
{
public:
    set<BookIPtr> m_books;

    void FinalReference(BaseObject *in_obj)
    {
        set<BookIPtr>::iterator it = m_books.find(BookIPtr((Book*)in_obj));
        if( it != m_books.end() )
        {
            in_obj->m_container = 0;
            m_books.erase( it );
        }
    }
};

namespace boost
{
    inline void intrusive_ptr_add_ref(BaseObject *p)
    {
        // increment reference count of object *p
        p->AddRef();
    }
    inline void intrusive_ptr_release(BaseObject *p)
    {
        // decrement reference count, and delete object when reference count reaches 0
        p->Release();
    } 
} // namespace boost

Cheers Rich

+7  A: 

I never used boost::intrusive smart pointers, but if you would use shared_ptr smart pointers, you could use weak_ptr objects for your cache.

Those weak_ptr pointers do not count as a reference when the system decides to free their memory, but can be used to retrieve a shared_ptr as long as the object has not been deleted yet.

Timbo
I have to use intrusive pointers as the code base is huge and very interconnected. I can't change the base type to a smart pointer. I also have to be able to change an existing pointer into a smart pointer and maintain the reference count. Something that intrusive pointers can do.
Rich
With shared pointers, you won't need the base type at all, which will be a good step towards disentangling your code base. You'll also get thread safety and exception safety.
Mike Seymour
The largest problem with shared pointer is that I would need to propergate the use of the shared type. I have to be able to use the basic c pointer as well, there is simply too much code to convert.
Rich
+3  A: 

You can use boost shared_ptr. With this you can provide a custom deleter (see this SO thread on how to do it). And in that custom deleter you know that you have reached the last reference count. You can now remove the pointer from the cache.

Abhay
Can you add custom deleters to intrustive pointers? (I know I can by writing my own version of intrusive pointers)
Rich
As long as the pointer is stored in the cache, the reference count will never drop to 0 and the custom deleter won't be called.
visitor
@visitor: Yes you are right, but the OP can re-design the cache mechanism. Let the cache contain raw pointers which will be wrapped by the `shared_ptr` before being accessed by the GUI windows. When all the pointers go out of scope, the user can remove the cache entry and also delete the same if needed.
Abhay
@Abhay: better still, store weak pointers (as Timbo suggests) to avoid dangling references.
Mike Seymour
@Rich: the `Release` function is essentially a custom deleter.
Mike Seymour
@Mike: Agreed. But how will you know when to remove it from cache?
Abhay
+1  A: 

You need to keep in your cache weak pointers instead of shared_ptr.

Tadeusz Kopec
A: 

You might consider writing an intrusive_weak_ptr for your cache class. You will still need to do something to clean out the expired weak pointers in your cache from time to time, but it's not as critical as cleaning up the actual cached objects.

http://lists.boost.org/boost-users/2008/08/39563.php is an implementation that was posted to the boost mailing list. It isn't thread safe, but it might work for you.

christopher_f