[Gc] note on GC_wait_marker() for Win32

Ivan Maidanski ivmai at mail.ru
Thu Nov 13 11:07:32 PST 2008


A piece of code:
    /* FIXME: This looks dubious to me.  Is it really OK if     */
    /* GC_mark_mutex_waitcnt is decremented between the         */
    /* following two lines?- HB                                 */
    if ((waitcnt = AO_load(&GC_mark_mutex_waitcnt)) > 1) {
    } else {
        GC_ASSERT(waitcnt != 0);

1. GC_mark_mutex_waitcnt is decremented only by the mutex owner (so, it is really OK);
2. AO_fetch_and_sub1_full() should be replaced with "release" version (since that's what we are doing in GC_release_mark_lock());
3. AO_load() (atomicity for getting GC_mark_mutex_waitcnt value) is not needed here - not easy to explain but I try - "GC_mark_mutex_waitcnt > 1" comparison is to filter out the case when nobody else is acquiring the lock - it's not a problem if someone starts acquiring the lock between we begin to read GC_mark_mutex_waitcnt value and finish reading it (or invoke fetch_and_sub1) - if we erroneously get value <= 1 (when real value is bigger) then this results in just one extra WaitForSingleObject() call (and no harm), we can't erroneously get value>1 when nobody else is trying to acquire the lock at this moment (i.e. doing incrementation of GC_mark_mutex_waitcnt).
4. If we assume MAX_MARKERS may be > 255 (in the far future!) then "GC_ASSERT(waitcnt != 0)" should either be changed to "GC_ASSERT(AO_load(&GC_mark_mutex_waitcnt) != 0)" or just be removed (otherwise we may get inconsistent zero value (not possible on all_aligned_atomic_load_store CPU) when someone else doing incrementation of GC_mark_mutex_waitcnt == 0xff (to 0x100)).


More information about the Gc mailing list