649920 - AsymmGC - Breaking the 'lock, allocate, wait-for-collection' deadlock

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Garbage Collection (mmGC), defect)

Description

[Simon Wilkinson]

This patch has been factored out from bug 624023.

It is the second of two patches that provide special-case synchronization constructs for use by asymmGC mutators.

(This is the same background paragraph as appears in bug 649895).
It is an invariant forced upon asymmGC that only the gc-owner thread may perform the FinishIncrementalMark collection phase. This phase is triggered during allocation when a mutator's minor allocation budget is spent and there is no more major allocation budget from which to refill it. If a non-owner mutator (VM-thread) triggers this collection phase then it must *wait* for the gc-owner thread to perform the collection. The gc-owner thread 'recognizes' that such a collection must be performed by either exhausting its own minor allocation budget or by inspection of GC::m_hasCollectionWaiter at GC entry and exit points. For the common-case, this arrangement is perfectly fine. The question remains, however, of what happens if a gc-owner thread is *blocked* by a non-owner mutator that subsequently requires the gc-owner thread perform a collection. In this case the non-owner mutator will wait forever for the gc-owner thread. I.e. we have deadlock.

There are two distinct scenarios that lead to the deadlock described above:

1) If a non-owner mutator attempts an allocation whilst holding a lock that can block the gc-owner thread.

2) If a non-owner mutator attempts an allocation whilst the gc-owner thread is waiting on some notification from the non-owner mutator.

This patch deals with scenario (1) described above. 

The general idea is to allow a non-owner mutator to temporarily prevent its allocations from triggering FinishIncrementalMark. It need only do this whilst holding a lock that can block the gc-owner thread. The GCAutoCollectionDisable class provides a RAII pattern for this functionality. This is clearly not a solution that should be used often, or where a thread could severely blow its allocation budget. The preferred solution is to just avoid the deadlock scenario occurring in the first-place.

So far I have only needed to use this in one instance: to enable a String to be allocated when interning some other String (the intern-table is protected by a lock). See bug 582776.

The general pattern is:

Foo* foo() {
    // This lock can be contended by mutators
    SCOPE_LOCK(m_lock) {
        if (uncommon-case) {
            GCAutoCollectionDisable gcDisable(gc);
            return new (gc) Foo();
        } else {
            // No allocations on this path
            return NULL;
        }
    }
}

Comment 1

[Simon Wilkinson]

Attachment: Latest. TR rev 6090. Patch queue rev 276

Comment 2

[Dan Smith]

Retargeting to Dolores.