Dave Mitchell <davem@[...].com>  wrote:
:On Wed, Mar 02, 2005 at 04:08:38AM +0000, hv@[...].org wrote:
:>  Hmm, locking a 'refcount--' doesn't make much sense to me: if two threads
:>  both do this at the same time, they'll still then both try to free the
:>  structure. But I see that op_free() does essentially the same thing,
:>  which confuses me. Can anyone explain how that makes sense?
:
:I haven't been following this thread closely (I don't even know what a
:trie is), but I'll chip in anyway.
:
:In the op_free() case, an op tree may be shared by CVs from differnt threads.
:The head of the tree contains an OP refcnt, so OP_REFCNT_LOCK is used to
:lock it before decrementing it.

Right, looking at the current usage again I see my confusion: in the op.c
case the lock protects not just the decrement itself, but crucially also
reading the resulting refcount:
            OP_REFCNT_LOCK;
            refcnt = OpREFCNT_dec(o);
            OP_REFCNT_UNLOCK;
            if (refcnt) [...]
So the trie patch needs to replace:
                        OP_REFCNT_LOCK;
                        trie-> refcount--;
                        OP_REFCNT_UNLOCK;
                        if (!trie-> refcount) {
with:
                        U32 refcount;
                        OP_REFCNT_LOCK;
                        refcount = --trie-> refcount;
                        OP_REFCNT_UNLOCK;
                        if (!refcount) {
to ensure that competing threads don't both try to free the same structure.
Beyond that there is the separate question of whether PL_op_mutex is the
right mutex to be locking on: I don't see substantial harm to it, but it
could be confusing for archaeologists to come.

Hugo