| Title: | MPS AMC pool + auto_header format: nailboards leak ControlPool memory |
| Status: | closed |
| Priority: | critical |
| Assigned user: | Richard Kistruck |
| Product: | mps |
| 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/branch/2008-03-11/auto_header/code/log/20080311t193718-amcsshe.txt FIXED http://info.ravenbrook.com/project/mps/branch/2008-03-11/auto_header/code/log/20080311t194336-amcsshe.txt 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 found: | inspection |
| Evidence: | http://info.ravenbrook.com/project/mps/master/code/poolamc.c#26 http://info.ravenbrook.com/project/mps/branch/2008-03-11/auto_header/code/log/20080311t193718-amcsshe.txt http://info.ravenbrook.com/project/mps/branch/2008-03-11/auto_header/code/log/20080311t194336-amcsshe.txt |
| Observed in: | 1.108.1 |
| Introduced in: | 1.100.0 |
| Test procedure: | none |
| Created by: | Richard Kistruck |
| Created on: | 2008-03-07 18:17:38 |
| Last modified by: | Richard Kistruck |
| Last modified on: | 2008-03-31 16:26:36 |
| 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). |
Generated at 2008-10-14 19:29:57 by $Id: //info.ravenbrook.com/infosys/cgi/issue.cgi#430 $
Copyright © Ravenbrook Limited. This document is provided "as is", without any express or implied warranty. In no event will the authors be held liable for any damages arising from the use of this document. You may not duplicate or reproduce this document in any form without the express permission of the copyright holder.