513906 - Fix Decommit logic to handle vm_map recommit failure of partial block

Status: VERIFIED FIXED

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

Description

[Tommy Reilly]

Need to split into separate regions and forget about the block we couldn't recommit.

Comment 1

[Lars T Hansen]

See also bug #520080 - that too calls for an abstraction layer around commit/decommit.

Comment 2

[Tommy Reilly]

Attachment: implements RemovePartialBlock

Comment 3

[Lars T Hansen]

The use of "#if AVMSYSTEM_MAC" in GCHeap.h and GCHeap.cpp is incorrect; we must never test these names in normal code.  "#ifdef MMGC_MAC" is what you want, since we're still using the MMGC_ prefix names in MMgc code.  Otherwise AVMPLUS_MAC.  See avmfeatures.as.

Please capitalize and punctuate the comment above the prototype for RemovePartialBlock in GCHeap.h.

Any particular reason the Region constructor needs to be inline?

In GCHeap.cpp, around line 1690 (the call to NewRegion), the new comment needs capitalization and punctuation.  There is also a half-finished comment just before the else that must be completed.

In RemovePartialBlock, the 'goto' loop is really a 'while' loop (with the identical condition) and should be written as such.

In RemovePartialBlock, the variable 'newRegion' is never used.

Comment 4

[Lars T Hansen]

Comment on attachment 416453 [details] [diff] [review]
implements RemovePartialBlock

Approved, provided changes are made to address those concerns.

Comment 5

[Tommy Reilly]

fixed in tr 3291

Comment 6

[Chris Peyer]

Engineering work item.  Marking as verified.