| Title | MPS AMC pool + auto_header format: nailboards leak ControlPool memory |
| Status | closed |
| Priority | essential |
| Assigned user | Richard Kistruck |
| Organization | Ravenbrook |
| Description | MPS 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. Impact: - 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 mps_arena_destroy(). Fixed: correctly release all of this temporary memory. |
| Analysis | RHSK 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: ORIGINAL http://info.ravenbrook.com/project/mps...er/code/log/20080311t193718-amcsshe.txtFIXED http://info.ravenbrook.com/project/mps...er/code/log/20080311t194336-amcsshe.txtRHSK 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 found | inspection |
| Evidence | http://info.ravenbrook.com/project/mps/master/code/poolamc.c#26http://info.ravenbrook.com/project/mps...er/code/log/20080311t193718-amcsshe.txthttp://info.ravenbrook.com/project/mps...er/code/log/20080311t194336-amcsshe.txt |
| Observed in | 1.108.1 |
| Introduced in | 1.100.0 |
| Created by | Richard Kistruck |
| Created on | 2008-03-07 18:17:38 |
| Last modified by | Gareth Rees |
| Last modified on | 2014-04-13 09:59:55 |
| History | 2008-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 test_runner.py (as it did before the fix). |