Bug in ReferenceCountedArrays and thread-use


#1

Jules, I’m getting assertions in RefCountedObject::incReferenceCount() and I wonder if there’s a subtle bug.

This is on xcode latest OS. The debugger shows the refcount as: -1009623872 or 0xc3d25cc0, so the assertion’s understandable.

I’m wondering if I have access to an element one after the end of my array. I’m trying hard to avoid extra locking, so I’m doing this:

[code]for (int i = 0; i < frames.size(); ++i)
{
ReferenceCountedObjectPtr next = frames[i];

if (next != NULL)
{

[/code]

The idea is that even if the size changed in between the loop check and the access, I’ll just get a NULL. This does seem to be that edge case - numUsed is 12 and i is 12 also, so that’s one of the end of the array, but I seem to get the item and try to refcount it, with predictable results. The ‘frames’ array does have a CriticalSection, BTW.

So, if the last array element was removed (moved, really) in between the size check and access, what could have happened?

Looking around, is it possible for the array to have been decremented while the value is being passed back? I see that operator[] unlocks before returning. Oh, wait - it looks like it’s returning an ObjectClass* not a refcountedPtr. Is that why?

It seems like it to me - It doesn’t look like RefCountedPtrs can be pulled off an array in a thread-safe way, since the receiver’s constructor is relied on to increment, and that leaves a gap for all sorts of evilness.

Any clues to fix that? Will existing uses break if it starts returning refctPtrs not pointers to an object that may have been erased or overwritten.

Bruce


#2

Well, unless your array is set up to use locking, the operator[] method isn’t actually thread-safe, so could easily return rubbish if another thread is messing with it at the same time…


#3

My array is set to use a CriticalSection. Did you look at the code? What happens is this:

[code]Thread A Thread B
operator[12] - gets element 12

  • returns pointer to element 12
    moves element 12 to 11
    receives pointer and constructs RefCtPtr
    increment fails, as 12 has now moved (hopefully it fails)[/code]

When I locked around all [] access, the assertions stopped. It seems like [] at least should return a refcountedptr somehow, with the ref count already incremented, or else it’s bound to fail in some circumstances.

Currently, you can put things into a ReferenceCountedArray safely, but there’s no way to get them out (i.e access them at all) in a threadsafe way without external locking.

Bruce


#4

Ah, sorry, I see what you mean now… Yes, that’s a very good point - it really should return a refcountedobjectptr instead of the raw pointer. I’ll sort that out right away!