Re: gvariant threading snafu ?

[Michael Meeks <michael meeks novell com>]

Hi Ryan,

On Thu, 2010-02-25 at 18:17 -0500, Ryan Lortie wrote:
> Thanks for catching this.  This is truly an impressive bug.  I'm
> surprised you were able to track it down -- I hope it didn't take too
> much time. :)

	Lol - my machine has a death-wish obviously.

> I fixed the issue you caught along with another closely-related one.
> That patch is now on glib master.

	Thanks.

> GObject weak references are actually not thread-safe, due to a very
> similar problem.  See https://bugzilla.gnome.org/show_bug.cgi?id=548954

	Oh - ok - but there is enough code in g_object_unref that they ought to
be ;-)

> I understand from your following message that the other issue is not a
> glib bug?

	Right; just a silly not turning thread safety on.

	FWIW; I think we can do a little better here with atomic ref-counting;
of course - beware, Meeks code - but it's been in use in OO.o for quite
some time without any obvious snafus (just like the I/O calls that don't
check for EINTR etc. ;-).

	This is a similar situation; interning ref-counted strings, whose
references are held in noddy hand-coded hash-table (to ensure we have a
unique version):

static void
internRelease (rtl_uString *pThis)
{
    oslMutex pPoolMutex;

    rtl_uString *pFree = NULL;
    if ( SAL_STRING_REFCOUNT(
             osl_decrementInterlockedCount( &(pThis->refCount) ) ) == 0)
    {
        pPoolMutex = getInternMutex();
        osl_acquireMutex( pPoolMutex );

        rtl_str_hash_remove (pInternPool, pThis);

        /* May have been separately acquired */
        if ( SAL_STRING_REFCOUNT(
                 osl_incrementInterlockedCount( &(pThis->refCount) ) ) == 1 )
        {
            /* we got the last ref */
            pFree = pThis;
        }
        else /* very unusual */
        {
            internRelease (pThis);
        }

        osl_releaseMutex( pPoolMutex );
    }
    if (pFree)
        rtl_freeMemory (pFree);
}

	The slightly non-obvious thing is that the recursion can only happen
once due to the recursive pool mutex we still hold; such that the 'very
unusual' unref(release) should only happen once; since in the worst case
we would be racing ourselves to unref, and the other caller would be
blocking in the same method. And of course, with interning - it's a lazy
optimisation so if we miss a few hits, it doesn't matter.

	HTH,

		Michael.

-- 
 michael meeks novell com  <><, Pseudo Engineer, itinerant idiot