From f8e860999ad271d1f965cad23694b4496305049f Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Sun, 12 Aug 2012 10:55:37 -0600 Subject: [PATCH] fix incorrect reporting of fixie collection status in heap.cpp This led to fixed-position objects being considered unreachable when they were actually still reachable, causing global weak JNI references to be cleared prematurely, most notably leading to crashes in AWT buffered image code. This commit also fixes a field offset calculation mismatch in bootimage.cpp relative to machine.cpp. --- src/bootimage.cpp | 18 +++---- src/heap.cpp | 122 +++++++++++++++++++++++++++++++++++++--------- src/heap.h | 1 + src/machine.cpp | 2 + 4 files changed, 107 insertions(+), 36 deletions(-) diff --git a/src/bootimage.cpp b/src/bootimage.cpp index a4ac6331c6..4525b3ec05 100644 --- a/src/bootimage.cpp +++ b/src/bootimage.cpp @@ -34,7 +34,7 @@ const unsigned TargetFixieSizeInBytes = 8 + (TargetBytesPerWord * 2); const unsigned TargetFixieSizeInWords = ceiling (TargetFixieSizeInBytes, TargetBytesPerWord); const unsigned TargetFixieAge = 0; -const unsigned TargetFixieHasMask = 1; +const unsigned TargetFixieFlags = 2; const unsigned TargetFixieSize = 4; const bool DebugNativeTarget = false; @@ -441,11 +441,7 @@ makeCodeImage(Thread* t, Zone* zone, BootImage* image, uint8_t* code, memberFields[memberIndex] = *f; - while (targetMemberOffset % f->targetSize) { - ++ targetMemberOffset; - } - - targetMemberOffset += f->targetSize; + targetMemberOffset = f->targetOffset + f->targetSize; ++ memberIndex; } @@ -540,8 +536,6 @@ makeCodeImage(Thread* t, Zone* zone, BootImage* image, uint8_t* code, } } - // if (strcmp(name, "avian/VMClass.class") == 0) trap(); - if (hashMapFind(t, typeMaps, c, objectHash, objectEqual) == 0) { object array = makeByteArray (t, TypeMap::sizeInBytes @@ -1175,13 +1169,13 @@ makeHeapImage(Thread* t, BootImage* image, target_uintptr_t* heap, memset(heap + position, 0, TargetFixieSizeInBytes); - uint8_t age = FixieTenureThreshold + 1; + uint16_t age = targetV2(FixieTenureThreshold + 1); memcpy(reinterpret_cast(heap + position) - + TargetFixieAge, &age, 1); + + TargetFixieAge, &age, 2); - uint8_t hasMask = true; + uint16_t flags = targetV2(1); memcpy(reinterpret_cast(heap + position) - + TargetFixieHasMask, &hasMask, 1); + + TargetFixieFlags, &flags, 2); uint32_t targetSize = targetV4(size); memcpy(reinterpret_cast(heap + position) diff --git a/src/heap.cpp b/src/heap.cpp index ed5d8777d4..0bc37862fd 100644 --- a/src/heap.cpp +++ b/src/heap.cpp @@ -463,12 +463,15 @@ class Segment { class Fixie { public: + static const unsigned HasMask = 1 << 0; + static const unsigned Marked = 1 << 1; + static const unsigned Dirty = 1 << 2; + static const unsigned Dead = 1 << 3; + Fixie(Context* c, unsigned size, bool hasMask, Fixie** handle, bool immortal): age(immortal ? FixieTenureThreshold + 1 : 0), - hasMask(hasMask), - marked(false), - dirty(false), + flags(hasMask ? HasMask : 0), size(size), next(0), handle(0) @@ -536,16 +539,54 @@ class Fixie { } unsigned totalSize() { - return totalSize(size, hasMask); + return totalSize(size, hasMask()); + } + + bool hasMask() { + return (flags & HasMask) != 0; + } + + bool marked() { + return (flags & Marked) != 0; + } + + void marked(bool v) { + if (v) { + flags |= Marked; + } else { + flags &= ~Marked; + } + } + + bool dirty() { + return (flags & Dirty) != 0; + } + + void dirty(bool v) { + if (v) { + flags |= Dirty; + } else { + flags &= ~Dirty; + } + } + + bool dead() { + return (flags & Dead) != 0; + } + + void dead(bool v) { + if (v) { + flags |= Dead; + } else { + flags &= ~Dead; + } } // be sure to update e.g. TargetFixieSizeInBytes in bootimage.cpp if // you add/remove/change fields in this class: - uint8_t age; - uint8_t hasMask; - uint8_t marked; - uint8_t dirty; + uint16_t age; + uint16_t flags; uint32_t size; Fixie* next; Fixie** handle; @@ -850,11 +891,11 @@ free(Context* c, Fixie** fixies, bool resetImmortal) fprintf(stderr, "reset immortal fixie %p\n", f); } *p = f->next; - memset(f->mask(), 0, Fixie::maskSize(f->size, f->hasMask)); + memset(f->mask(), 0, Fixie::maskSize(f->size, f->hasMask())); f->next = 0; f->handle = 0; - f->marked = false; - f->dirty = false; + f->marked(false); + f->dirty(false); } else { p = &(f->next); } @@ -868,6 +909,28 @@ free(Context* c, Fixie** fixies, bool resetImmortal) } } +void +kill(Fixie* fixies) +{ + for (Fixie* f = fixies; f; f = f->next) { + if (! f->immortal()) { + f->dead(true); + } + } +} + +void +killFixies(Context* c) +{ + assert(c, c->markedFixies == 0); + + if (c->mode == Heap::MajorCollection) { + kill(c->tenuredFixies); + kill(c->dirtyTenuredFixies); + } + kill(c->fixies); +} + void sweepFixies(Context* c) { @@ -898,14 +961,14 @@ sweepFixies(Context* c) if (f->age >= FixieTenureThreshold) { if (DebugFixies) { - fprintf(stderr, "tenure fixie %p (dirty: %d)\n", f, f->dirty); + fprintf(stderr, "tenure fixie %p (dirty: %d)\n", f, f->dirty()); } if (not f->immortal()) { c->tenuredFixieFootprint += f->totalSize(); } - if (f->dirty) { + if (f->dirty()) { f->add(c, &(c->dirtyTenuredFixies)); } else { f->add(c, &(c->tenuredFixies)); @@ -916,7 +979,7 @@ sweepFixies(Context* c) f->add(c, &(c->fixies)); } - f->marked = false; + f->marked(false); } c->tenuredFixieCeiling = max @@ -1006,14 +1069,14 @@ update3(Context* c, void* o, bool* needsVisit) { if (c->client->isFixed(o)) { Fixie* f = fixie(o); - if ((not f->marked) + if ((not f->marked()) and (c->mode == Heap::MajorCollection or f->age < FixieTenureThreshold)) { if (DebugFixies) { fprintf(stderr, "mark fixie %p\n", f); } - f->marked = true; + f->marked(true); f->move(c, &(c->markedFixies)); } *needsVisit = false; @@ -1044,13 +1107,13 @@ update2(Context* c, void* o, bool* needsVisit) void markDirty(Context* c, Fixie* f) { - if (not f->dirty) { + if (not f->dirty()) { #ifdef USE_ATOMIC_OPERATIONS ACQUIRE(c->lock); #endif - if (not f->dirty) { - f->dirty = true; + if (not f->dirty()) { + f->dirty(true); f->move(c, &(c->dirtyTenuredFixies)); } } @@ -1059,8 +1122,8 @@ markDirty(Context* c, Fixie* f) void markClean(Context* c, Fixie* f) { - if (f->dirty) { - f->dirty = false; + if (f->dirty()) { + f->dirty(false); if (f->immortal()) { f->remove(c); } else { @@ -1090,7 +1153,7 @@ updateHeapMap(Context* c, void* p, void* target, unsigned offset, void* result) { if (target and c->client->isFixed(target)) { Fixie* f = fixie(target); - assert(c, offset == 0 or f->hasMask); + assert(c, offset == 0 or f->hasMask()); if (static_cast(f->age + 1) >= FixieTenureThreshold) { if (DebugFixies) { @@ -1098,7 +1161,7 @@ updateHeapMap(Context* c, void* p, void* target, unsigned offset, void* result) f, offset, f->body() + offset, result); } - f->dirty = true; + f->dirty(true); markBit(f->mask(), offset); } } else if (seg->contains(p)) { @@ -1871,7 +1934,7 @@ class MyHeap: public Heap { if (c.client->isFixed(p)) { Fixie* f = fixie(p); - assert(&c, offset == 0 or f->hasMask); + assert(&c, offset == 0 or f->hasMask()); bool dirty = false; for (unsigned i = 0; i < count; ++i) { @@ -1946,11 +2009,22 @@ class MyHeap: public Heap { } } + virtual void postVisit() { + killFixies(&c); + } + virtual Status status(void* p) { p = mask(p); if (p == 0) { return Null; + } else if (c.client->isFixed(p)) { + Fixie* f = fixie(p); + return f->dead() + ? Unreachable + : (static_cast(f->age + 1) < FixieTenureThreshold + ? Reachable + : Tenured); } else if (c.nextGen1.contains(p)) { return Reachable; } else if (c.nextGen2.contains(p) diff --git a/src/heap.h b/src/heap.h index 875b4a5cbc..c3f1f8b4c8 100644 --- a/src/heap.h +++ b/src/heap.h @@ -69,6 +69,7 @@ class Heap: public Allocator { virtual void mark(void* p, unsigned offset, unsigned count) = 0; virtual void pad(void* p) = 0; virtual void* follow(void* p) = 0; + virtual void postVisit() = 0; virtual Status status(void* p) = 0; virtual CollectionType collectionType() = 0; virtual void disposeFixies() = 0; diff --git a/src/machine.cpp b/src/machine.cpp index dbf2e12dfc..b9782ac3f0 100644 --- a/src/machine.cpp +++ b/src/machine.cpp @@ -2690,6 +2690,8 @@ class HeapClient: public Heap::Client { virtual void visitRoots(Heap::Visitor* v) { ::visitRoots(m, v); + m->heap->postVisit(); + postVisit(m->rootThread, v); }