[kaffe] CVS kaffe (hkraemer): fixed mem leak in garbage collector
Kaffe CVS
cvs-commits at kaffe.org
Fri Aug 22 04:44:02 PDT 2003
PatchSet 3971
Date: 2003/08/22 11:42:13
Author: hkraemer
Branch: HEAD
Tag: (none)
Log:
fixed mem leak in garbage collector
Members:
ChangeLog:1.1568->1.1569
kaffe/kaffevm/mem/gc-incremental.c:1.66->1.67
kaffe/kaffevm/mem/gc-mem.c:1.45->1.46
Index: kaffe/ChangeLog
diff -u kaffe/ChangeLog:1.1568 kaffe/ChangeLog:1.1569
--- kaffe/ChangeLog:1.1568 Fri Aug 22 09:02:39 2003
+++ kaffe/ChangeLog Fri Aug 22 11:42:13 2003
@@ -1,3 +1,13 @@
+2003-08-22 Helmer Kraemer <hkraemer at freenet.de>
+
+ * kaffe/kaffevm/mem/gc-mem.c:
+ (gc_heap_free) properly free empty primitive blocks
+
+ * kaffe/kaffevm/mem/gc-incremental.c:
+ (gcWalkMemory, startGC, gcMan, finishGC,gcMalloc,createGC)
+ manage objects that have a finalizer and objects that don't
+ have one in different white and black lists
+
2003-08-22 Dalibor Topic <robilad at kaffe.org>,
* libraries/javalib/Klasses.jar.bootstrap:
Index: kaffe/kaffe/kaffevm/mem/gc-incremental.c
diff -u kaffe/kaffe/kaffevm/mem/gc-incremental.c:1.66 kaffe/kaffe/kaffevm/mem/gc-incremental.c:1.67
--- kaffe/kaffe/kaffevm/mem/gc-incremental.c:1.66 Sat Jul 26 16:50:51 2003
+++ kaffe/kaffe/kaffevm/mem/gc-incremental.c Fri Aug 22 11:42:15 2003
@@ -47,11 +47,12 @@
Hjava_lang_Thread* garbageman;
static Hjava_lang_Thread* finalman;
-static gcList gclists[5];
-static const int mustfree = 4; /* temporary list */
-static const int white = 3;
-static const int grey = 2;
-static const int black = 1;
+static gcList gclists[6];
+static const int nofin_white = 5;
+static const int fin_white = 4;
+static const int grey = 3;
+static const int nofin_black = 2;
+static const int fin_black = 1;
static const int finalise = 0;
static int gc_init = 0;
@@ -430,8 +431,26 @@
info = GCMEM2BLOCK(unit);
idx = GCMEM2IDX(info, unit);
+ if (GC_GET_COLOUR(info, idx) == GC_COLOUR_BLACK) {
+ return;
+ }
+
UREMOVELIST(unit);
- UAPPENDLIST(gclists[black], unit);
+
+ /* if the object is about to be finalized, put it directly
+ * into the finalise list, otherwise put it into the black
+ * list.
+ */
+ if (GC_GET_STATE(info, idx) == GC_STATE_INFINALIZE) {
+ gcStats.finalobj += 1;
+ gcStats.finalmem += GCBLOCKSIZE(info);
+ UAPPENDLIST(gclists[finalise], unit);
+ } else if (GC_GET_STATE(info, idx) == GC_STATE_NEEDFINALIZE) {
+ UAPPENDLIST(gclists[fin_black], unit);
+ } else {
+ UAPPENDLIST(gclists[nofin_black], unit);
+ }
+
GC_SET_COLOUR(info, idx, GC_COLOUR_BLACK);
assert(GC_GET_FUNCS(info, idx) <
@@ -474,7 +493,6 @@
gcMan(void* arg)
{
gc_unit* unit;
- gc_unit* nunit;
gc_block* info;
int idx;
Collector *gcif = (Collector*)arg;
@@ -554,27 +572,33 @@
startGC(gcif);
- for (unit = gclists[grey].cnext; unit != &gclists[grey]; unit = gclists[grey].cnext) {
+ /* process any objects found by walking the root references */
+ while (gclists[grey].cnext != &gclists[grey]) {
+ unit = gclists[grey].cnext;
gcWalkMemory(gcif, UTOMEM(unit));
}
+
/* Now walk any white objects which will be finalized. They
* may get reattached, so anything they reference must also
* be live just in case.
*/
- for (unit = gclists[white].cnext; unit != &gclists[white]; unit = nunit) {
- nunit = unit->cnext;
+ while (gclists[fin_white].cnext != &gclists[fin_white]) {
+ unit = gclists[fin_white].cnext;
info = GCMEM2BLOCK(unit);
idx = GCMEM2IDX(info, unit);
- if (GC_GET_STATE(info, idx) == GC_STATE_NEEDFINALIZE) {
- /* this assert is somewhat expensive */
- DBG(GCDIAG,
- assert(gc_heap_isobject(info, unit)));
- GC_SET_STATE(info, idx, GC_STATE_INFINALIZE);
- markObjectDontCheck(unit, info, idx);
- }
+
+ assert (GC_GET_STATE(info, idx) == GC_STATE_NEEDFINALIZE);
+
+ /* this assert is somewhat expensive */
+ DBG(GCDIAG,
+ assert(gc_heap_isobject(info, unit)));
+ GC_SET_STATE(info, idx, GC_STATE_INFINALIZE);
+ markObjectDontCheck(unit, info, idx);
}
- /* We may now have more grey objects, so walk them */
- for (unit = gclists[grey].cnext; unit != &gclists[grey]; unit = gclists[grey].cnext) {
+
+ /* now process the objects that are referenced by objects to be finalized */
+ while (gclists[grey].cnext != &gclists[grey]) {
+ unit = gclists[grey].cnext;
gcWalkMemory(gcif, UTOMEM(unit));
}
@@ -639,7 +663,6 @@
startGC(Collector *gcif)
{
gc_unit* unit;
- gc_unit* nunit;
gcStats.freedmem = 0;
gcStats.freedobj = 0;
@@ -663,9 +686,8 @@
startTiming(&gc_time, "gctime-scan");
/* Walk all objects on the finalizer list */
- for (unit = gclists[finalise].cnext;
- unit != &gclists[finalise]; unit = nunit) {
- nunit = unit->cnext;
+ while (gclists[finalise].cnext != &gclists[finalise]) {
+ unit = gclists[finalise].cnext;
gcMarkObject(gcif, UTOMEM(unit));
}
@@ -685,31 +707,57 @@
gc_unit* unit;
gc_block* info;
int idx;
+ gcList toRemove;
+ int i;
/* There shouldn't be any grey objects at this point */
assert(gclists[grey].cnext == &gclists[grey]);
+ if (gclists[nofin_white].cnext != &gclists[nofin_white]) {
+ toRemove.cnext = gclists[nofin_white].cnext;
+ toRemove.cprev = gclists[nofin_white].cprev;
+
+ toRemove.cnext->cprev = &toRemove;
+ toRemove.cprev->cnext = &toRemove;
+
+ URESETLIST(gclists[nofin_white]);
+ } else {
+ URESETLIST(toRemove);
+ }
+
+ stopTiming(&gc_time);
+
+ RESUMEWORLD();
+
/*
- * Any white objects should now be freed, but we cannot call
- * gc_heap_free here because we might block in gc_heap_free,
- * which would leave the white list unprotected.
- * So we move them to a 'mustfree' list from where we'll pull them
- * off later.
- *
- * XXX: this is so silly it hurts. Jason has a fix.
+ * Now move the black objects back to the white queue for next time.
*/
- while (gclists[white].cnext != &gclists[white]) {
- unit = gclists[white].cnext;
- UREMOVELIST(unit);
+ for (i=1; i<3; i++) {
+ while (gclists[i].cnext != &gclists[i]) {
+ unit = gclists[i].cnext;
+ UREMOVELIST(unit);
+
+ info = GCMEM2BLOCK(unit);
+ idx = GCMEM2IDX(info, unit);
+
+ assert(GC_GET_COLOUR(info, idx) == GC_COLOUR_BLACK);
+
+ UAPPENDLIST(gclists[i+3], unit);
+
+ GC_SET_COLOUR(info, idx, GC_COLOUR_WHITE);
+ }
+ }
+ startTiming(&sweep_time, "gctime-sweep");
+
+ while (toRemove.cnext != &toRemove) {
+ destroy_func_t destroy;
+ unit = toRemove.cnext;
info = GCMEM2BLOCK(unit);
idx = GCMEM2IDX(info, unit);
- assert(GC_GET_COLOUR(info, idx) == GC_COLOUR_WHITE);
- assert(GC_GET_STATE(info, idx) == GC_STATE_NORMAL);
gcStats.freedmem += GCBLOCKSIZE(info);
gcStats.freedobj += 1;
- UAPPENDLIST(gclists[mustfree], unit);
OBJECTSTATSREMOVE(unit);
#if defined(ENABLE_JVMPI)
@@ -722,45 +770,24 @@
jvmpiPostEvent(&ev);
}
#endif
- }
-
- /*
- * Now move the black objects back to the white queue for next time.
- * Note that all objects that were eligible for finalization are now
- * black - this is so because we marked and then walked them.
- * We recognize them by their "INFINALIZE" state, however, and put
- * them on the finalise list.
- */
- while (gclists[black].cnext != &gclists[black]) {
- unit = gclists[black].cnext;
- UREMOVELIST(unit);
+ /* invoke destroy function before freeing the object */
info = GCMEM2BLOCK(unit);
idx = GCMEM2IDX(info, unit);
-
- assert(GC_GET_COLOUR(info, idx) == GC_COLOUR_BLACK);
-
- if (GC_GET_STATE(info, idx) == GC_STATE_INFINALIZE) {
- gcStats.finalmem += GCBLOCKSIZE(info);
- gcStats.finalobj += 1;
- UAPPENDLIST(gclists[finalise], unit);
- }
- else {
- UAPPENDLIST(gclists[white], unit);
+ destroy = gcFunctions[GC_GET_FUNCS(info,idx)].destroy;
+ if (destroy != 0) {
+ destroy(gcif, UTOMEM(unit));
}
- GC_SET_COLOUR(info, idx, GC_COLOUR_WHITE);
- }
- /* this is where we'll stop locking out other threads
- * measure gc time until here. This is not quite accurate, as
- * it excludes the time to sweep objects, but lacking
- * per-thread timing it's a reasonable thing to do.
- */
- stopTiming(&gc_time);
+ UREMOVELIST(unit);
+ addToCounter(&gcgcablemem, "gcmem-gcable objects", 1,
+ -((jlong)GCBLOCKSIZE(info)));
+ gc_heap_free(unit);
+ }
#if defined(ENABLE_JVMPI)
if( JVMPI_EVENT_ISENABLED(JVMPI_EVENT_GC_FINISH) )
- {
+ {
JVMPI_Event ev;
ev.event_type = JVMPI_EVENT_GC_FINISH;
@@ -771,35 +798,6 @@
}
#endif
- /*
- * Now that all lists that the mutator manipulates are in a
- * consistent state, we can reenable the mutator here
- */
- RESUMEWORLD();
-
- /*
- * Now free the objects. We can block here since we're the only
- * thread manipulating the "mustfree" list.
- */
- startTiming(&sweep_time, "gctime-sweep");
-
- while (gclists[mustfree].cnext != &gclists[mustfree]) {
- destroy_func_t destroy;
- unit = gclists[mustfree].cnext;
-
- /* invoke destroy function before freeing the object */
- info = GCMEM2BLOCK(unit);
- idx = GCMEM2IDX(info, unit);
- destroy = gcFunctions[GC_GET_FUNCS(info,idx)].destroy;
- if (destroy != 0) {
- destroy(gcif, UTOMEM(unit));
- }
-
- UREMOVELIST(unit);
- addToCounter(&gcgcablemem, "gcmem-gcable objects", 1,
- -((jlong)GCBLOCKSIZE(info)));
- gc_heap_free(unit);
- }
stopTiming(&sweep_time);
}
@@ -1063,7 +1061,12 @@
* compiler to not delay computing mem by defining it volatile.
*/
GC_SET_COLOUR(info, i, GC_COLOUR_WHITE);
- UAPPENDLIST(gclists[white], unit);
+
+ if (GC_GET_STATE(info, i) == GC_STATE_NEEDFINALIZE) {
+ UAPPENDLIST(gclists[fin_white], unit);
+ } else {
+ UAPPENDLIST(gclists[nofin_white], unit);
+ }
}
if (!reserve) {
reserve = gc_primitive_reserve();
@@ -1388,11 +1391,12 @@
createGC(void (*_walkRootSet)(Collector*))
{
walkRootSet = _walkRootSet;
- URESETLIST(gclists[white]);
+ URESETLIST(gclists[nofin_white]);
+ URESETLIST(gclists[fin_white]);
URESETLIST(gclists[grey]);
- URESETLIST(gclists[black]);
+ URESETLIST(gclists[nofin_black]);
+ URESETLIST(gclists[fin_black]);
URESETLIST(gclists[finalise]);
- URESETLIST(gclists[mustfree]);
gc_obj.collector.ops = &GC_Ops;
return (&gc_obj.collector);
}
Index: kaffe/kaffe/kaffevm/mem/gc-mem.c
diff -u kaffe/kaffe/kaffevm/mem/gc-mem.c:1.45 kaffe/kaffe/kaffevm/mem/gc-mem.c:1.46
--- kaffe/kaffe/kaffevm/mem/gc-mem.c:1.45 Tue Jul 8 07:33:50 2003
+++ kaffe/kaffe/kaffevm/mem/gc-mem.c Fri Aug 22 11:42:15 2003
@@ -445,12 +445,13 @@
for (;*finfo;) {
if (*finfo == info) {
(*finfo) = info->next;
- info->size = gc_pgsize;
- gc_primitive_free(info);
break;
}
finfo = &(*finfo)->next;
}
+
+ info->size = gc_pgsize;
+ gc_primitive_free(info);
} else if (info->avail==1) {
/*
* If this block contains no free sub-blocks yet, attach
More information about the kaffe
mailing list