MPS issue job001784

TitleMPS AMC pool + auto_header format: nailboards leak ControlPool memory
Assigned userRichard Kistruck
DescriptionMPS AMC pool + auto_header format: nailboards leak ControlPool memory

RHSK 2008-03-07
If you have ambiguous references (typically on the stack) to objects
in an AMC segment when a collection starts, AMC allocates an internal
data structure (a "nailboard") to record them.

If the AMC pool is used with an auto_header format, a mistake in the
AMC pool code causes it to not deallocate all of this when the
collection ends.

Over time, this is a memory leak. Approximate cost per collection:
one byte per AMC segment (typically 4K) referred to by an ambiguous
reference, plus undesirable fragmentation of the control pool.

RHSK 2008-03-26
(as explained in readme.txt)
Defect discovered:
  - when using an auto_header format (mps_fmt_create_auto_header)
        with AMC pools (mps_class_amc), the MPS leaks a small amount of
        memory on each collection.
  - the leak is likely to be a few bytes per collection, and at most
        one byte per page (typically 2^12 bytes) of the address-space
        currently in use for objects in AMC pools;
  - the leak is of temporary memory that the MPS uses to process
        ambiguous references (typically references on the stack and in
        registers), so a larger stack when a collection starts will
        tend to cause a larger leak;
  - the leaked bytes are widely-spaced single bytes which therefore
        also cause fragmentation;
  - the leaked bytes are not reclaimed until the client calls
Fixed: correctly release all of this temporary memory.
AnalysisRHSK 2008-03-07
Nailboards are allocated from the control pool. If the client passed
an auto_header format when creating the pool, AMC allocates a
marginally larger nailboard according to the format's mps_headerSize
value. When AMC deallocates this nailboard, it fails to deallocate
this margin. This will typically leak one byte (if SegSize is a
nice round number).

The fix is trivial (don't forget the margin when calculating the size
of nailboard to free!).

RHSK 2008-03-11
See graphic diagnostics of leaked bytes:

RHSK 2008-03-12
The larger issue is that the entire auto_header format work has not
been subjected to scrutiny, and should be re-written to avoid the
scattering of error-prone arithmetic throughout the code of all
formatted pools.

But first, check for similar defects: add ArenaDestroy diag (@164385),
which shows the leaks persisting until ControlFinish is called.
Quick check shows no ControlPool leaks shown by either amssshe or
awluthe. Why are amcsshe/amssshe/awluthe *separate source files*
from amcss/amsss/awlut? The mpsicv test does not use auto_header
format. For now, hack it so it does.

RHSK 2008-03-31
The mpsicv test now uses auto_header half the time (at random).
(This is a useful improvement, but still would not have detected
this defect).
How foundinspection
Observed in1.108.1
Introduced in1.100.0
Created byRichard Kistruck
Created on2008-03-07 18:17:38
Last modified byGareth Rees
Last modified on2014-04-13 09:59:55
History2008-03-07 RHSK Created, from reading poolamc.c
2008-03-11 RHSK Show defect.
2008-03-26 RHSK Final fix; describe impact.
2008-03-31 RHSK note that mpsicv does now exercise auto_header


Change Effect Date User Description
164508 closed 2008-03-26 17:58:22 Richard Kistruck MPS integ from br/auto_header: job001784 AMC + auto_header leak
poolamc.c: [job001784] fix ControlPool leak in amcSegDestroyNailboard;
arena.c, mpm.h: new ControlDescribe() diagnostic function, to describe arena control pool;
mpsicv.c, comm.gmk: use auto_header format half the time (rnd() & 1).
+ readme.txt: describe job001784 fix.
164503 closed 2008-03-26 14:47:57 Richard Kistruck MPS br/auto_header: poolamc.c: [job001784] permanent fix: drop tabs, ifdef, and ephemeral diag; update copyright date
164396 open 2008-03-12 11:37:06 Richard Kistruck MPS br/auto_header: mpsicv can use auto_header format, by the magic
of adding #ifdefs... Ahem. I'll fix that in a minute.
164385 open 2008-03-12 08:29:46 Richard Kistruck MPS br/auto_header: Add ArenaDestroy diag: calls ControlPoolDescribe
just before finishing the control pool. This clearly shows the
job001784 leak, and allows some checking for other such leaks.
164383 open 2008-03-11 19:53:05 Richard Kistruck MPS br/auto_header:
Add diagnostics to show ControlPool leak when deallocating nailboards with auto_header format.
See logfiles added in this changelist.
164379 open 2008-03-11 19:18:50 Richard Kistruck MPS br/auto_header: trial fix of job001784 "AMC pool + auto_header format: nailboards leak ControlPool memory"
Passes (as it did before the fix).