542529 - Injection: Assert triggered when small-object allocation occurs during finalization

Status: VERIFIED FIXED

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

Description

[Lars T Hansen]

The slow branch of GCAlloc::AllocSlow calls GCAlloc::Alloc to perform the allocation after setting up a state that is suitable for alloc, but sets up an imperfect state: GCAlloc::Alloc has an invariant, that if gc->collecting is true then the quick list is empty.  AllocSlow either temporarily needs to set gc->collecting to false (probably OK given what we're doing but needs serious vetting, because of how mark bits are set etc - it's not appealing) or AllocSlow should not be calling Alloc recursively at that point.

We did not catch this in avmshell unit testing because we don't have a test that is sure to allocate during finalization.

Comment 1

[Lars T Hansen]

Attachment: Selftest that triggers the failure

Selftest that has a finalizer that allocates a small object, thereby triggering the assert in debug mode.

Comment 2

[Lars T Hansen]

Attachment: Patch

Splits GCAlloc::Alloc into a preamble and a new function, AllocFromQuickList, that actually picks the object off the list and returns it after setting everything up.  AllocSlow now calls the latter, which removes the reentrancy problem in this bug and the one for greedy mode as well.  The new function is made REALLY_INLINE so performance should not suffer.

Comment 3

[Tommy Reilly]

Comment on attachment 423813 [details] [diff] [review]
Patch

Looks good with exception of SignalFreeWork call no longer being necessary in AllocSlow

Comment 4

[Lars T Hansen]

Attachment: Revised patch, for posterity

Comment 5

[Lars T Hansen]

redux-argo changeset:   3625:45d0394f4ce3

Comment 6

[Brent Baker]

test added to selftest mmgc_basic in tr-argo 3623:acce1fef425e