Thread-safe reference counting in C++

[cjp]

I have a class which I want to have reference counting. So I added a private reference count variable, and the public methods incRef() and decRef(). The user is responsible for calling incRef and decRef. decRef automatically calls the destructor (with 'delete this') when the reference count reaches zero.

Now I want to make incRef and decRef thread-safe. My first idea is to add a critical section member to the class, and let both incRef and decRef lock the section on entering, and unlock on leaving the method.

One problem is that decRef may delete the object, so the critical section will no longer be available for unlocking. The safest, but not really efficient solution, which I use now for testing purposes, is to use a static critical section member, so that there effectively is a global lock on these operations.

Another problem is the following scenario:

• thread A calls decRef, and decRef locks the critical section
• thread B calls incRef, which attempts to lock, and blocks
• thread A finds a zero reference count, and deletes the object
• thread A unlocks the critical section
• thread B attempts to increase the reference count of a no longer existing object

I tried to detect whether this occurs, but I haven't seen it occur. OTOH, I do have another problem: the application locks on a completely different place. Based on the stack in the debugger, the place where it locks seems to be some completely unrelated multithreading code inside either SDL or OpenGL.

FYI: the purpose for all of this is to allow different terrain tiles in libProcTer to share the same crater objects.

 

[Urwumpe]

Well... you just need to prevent deletion if ANY other thread tries to reserve it. Maybe you should have a special counter variable for waiting "incRef" requests.

 

[cjp]

Is it a good idea to let incRef have a 'failure' result, if it turns out that that object was deleted before the reference could be incremented?

 

[Urwumpe]

Is it a good idea to let incRef have a 'failure' result, if it turns out that that object was deleted before the reference could be incremented?

Yes. Maybe it would be smarter to use smart pointers instead of directly handling the references, so you never call IncRef on a not-existing object.

 

[dbeachy1]

Also, it really isn't kosher to have a non-static member method delete the 'this' object -- you'd be destroying the object while one of the methods that rely on it is still running.

EDIT:
What about just creating a separate factory (or "pooler") class that each thread could use to acquire/release shared objects? The factory would manage all the locking/creation/deletion of each shared object. The factory class could even keep a pool of shared objects and then divvy them out as needed -- that way it wouldn't need to constantly create and destroy them. Of course, whether that's a good fit would depend on exactly what your shared objects need to do.

 

[cjp]

Yes. Maybe it would be smarter to use smart pointers instead of directly handling the references, so you never call IncRef on a not-existing object.

So, how would a smart pointer class handle this? Doesn't this just move the problem to the implementation of the smart pointer class?

Also, it really isn't kosher to have a non-static member method delete the 'this' object -- you'd be destroying the object while one of the methods that rely on it is still running.

Maybe not kosher, but I'm not Jewish . When searching for C++ "delete this", I found a website that listed four conditions that need to be satisfied to make this safe. I satisfy all four, but I agree it's still a bit ugly, because one of the conditions needs to be satisfied outside the class (objects shall only be created with new, and never on the stack, or with new[], or ...). Besides, have you ever heard of a static method doing 'delete this'?

 

[dbeachy1]

Besides, have you ever heard of a static method doing 'delete this'?

:lol: No, in that case you'd use:

delete pMyStaticObject;

...which looks much more kosher. :lol:

 

[Urwumpe]

So, how would a smart pointer class handle this? Doesn't this just move the problem to the implementation of the smart pointer class?

Yes, but you can check if the object is still valid BEFORE you access its members.

 

[cjp]

Of course, whether that's a good fit would depend on exactly what your shared objects need to do.

libProcTer splits up the ground into square tiles. I'm making a landscape generator for small 'crater-world' planets, moons and asteroids. To hold the data of each crater, I have a crater class. A crater is usually shared between different tiles: when it crosses the boundary between neighboring tiles, and when different levels of detail are loaded for the same piece of ground. Creating and deleting of tiles is often done in different threads.

So I have different tiles linking to the same crater objects, and I need to find out when the craters can be deleted safely. For that, I need the reference counting. Because I don't have nasty things like circular references, it is sufficient to delete all objects with reference count zero. But the multi-threading part is a bit harder than anticipated.

---------- Post added at 08:53 PM ---------- Previous post was at 08:36 PM ----------

I found a bug: an old piece of code where craters are deleted directly, instead of by calling decRef(). Could be the cause of my problem, but the issues I described still exist. I'll consider using some sort of factory object that keeps track of all crater objects.

The craters are really a sort of proof of concept for other feature-based scenery. A secondary goal is to make feature maintenance easy for future terrain plug-in developers for libProcTer.

---------- Post added at 10:16 PM ---------- Previous post was at 08:53 PM ----------

I found that the scenario I described earlier will never occur, and fixing the error in the code fixed the problem I was having. For now, I'm being lazy and I'll keep it this way, without solving the fundamental problem of multi-threaded reference counting.