From 9c9eb86b2feb6fbfe336fe40a1cf6962d42b606c Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Thu, 16 Jul 2009 11:51:35 -0600 Subject: [PATCH 1/3] fix deadlock in allocate3 when another thread wants to enter the exclusive state --- src/machine.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/machine.cpp b/src/machine.cpp index c0ee2be304..2fb5fbb1cc 100644 --- a/src/machine.cpp +++ b/src/machine.cpp @@ -2055,6 +2055,10 @@ allocate3(Thread* t, Allocator* allocator, Machine::AllocationType type, // another thread wants to enter the exclusive state, either for a // collection or some other reason. We give it a chance here. ENTER(t, Thread::IdleState); + + while (t->m->exclusive) { + t->m->stateLock->wait(t->systemThread, 0); + } } switch (type) { From 3e4336eba4e0bdf845f4372b03a4b98b2fd2b6f4 Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Fri, 17 Jul 2009 09:29:24 -0600 Subject: [PATCH 2/3] rearrange finalization code in collect to avoid inifinite recursion when finalizer allocates memory --- src/machine.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/machine.cpp b/src/machine.cpp index 2fb5fbb1cc..7843a1bb30 100644 --- a/src/machine.cpp +++ b/src/machine.cpp @@ -2762,13 +2762,6 @@ collect(Thread* t, Heap::CollectionType type) postCollect(m->rootThread); - for (object f = m->finalizeQueue; f; f = finalizerNext(t, f)) { - void (*function)(Thread*, object); - memcpy(&function, &finalizerFinalize(t, f), BytesPerWord); - function(t, finalizerTarget(t, f)); - } - m->finalizeQueue = 0; - killZombies(t, m->rootThread); for (unsigned i = 0; i < m->heapPoolIndex; ++i) { @@ -2781,6 +2774,14 @@ collect(Thread* t, Heap::CollectionType type) #ifdef VM_STRESS if (not stress) t->stress = false; #endif + + object f = m->finalizeQueue; + m->finalizeQueue = 0; + for (; f; f = finalizerNext(t, f)) { + void (*function)(Thread*, object); + memcpy(&function, &finalizerFinalize(t, f), BytesPerWord); + function(t, finalizerTarget(t, f)); + } } void From 47ab9805502cb7b0784accf72f0767d2668c1657 Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Fri, 17 Jul 2009 19:37:46 -0600 Subject: [PATCH 3/3] fix thread heap overflow corner case in allocate3 The previous code relied on the invalid assumption that the thread-local heaps for all threads would have been cleared immediately following a garbage collection. However, the last thing the garbage collection function does is run finalizers which may allocate new objects. This can lead allocate3 to call allocateSmall with a size which is too large to accomodate, overflowing the heap. The solution is to iterate until there really is enough room for the original allocation request. --- src/machine.cpp | 63 ++++++++++++++++++++++++++----------------------- src/machine.h | 3 +++ 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/machine.cpp b/src/machine.cpp index 7843a1bb30..9b8ddb7c38 100644 --- a/src/machine.cpp +++ b/src/machine.cpp @@ -2061,42 +2061,47 @@ allocate3(Thread* t, Allocator* allocator, Machine::AllocationType type, } } - switch (type) { - case Machine::MovableAllocation: - if (t->heapIndex + ceiling(sizeInBytes, BytesPerWord) - > ThreadHeapSizeInWords) - { - t->heap = 0; - if (t->m->heapPoolIndex < ThreadHeapPoolSize) { - t->heap = static_cast - (t->m->heap->tryAllocate(ThreadHeapSizeInBytes)); + do { + switch (type) { + case Machine::MovableAllocation: + if (t->heapIndex + ceiling(sizeInBytes, BytesPerWord) + > ThreadHeapSizeInWords) + { + t->heap = 0; + if (t->m->heapPoolIndex < ThreadHeapPoolSize) { + t->heap = static_cast + (t->m->heap->tryAllocate(ThreadHeapSizeInBytes)); - if (t->heap) { - memset(t->heap, 0, ThreadHeapSizeInBytes); + if (t->heap) { + memset(t->heap, 0, ThreadHeapSizeInBytes); - t->m->heapPool[t->m->heapPoolIndex++] = t->heap; - t->heapOffset += t->heapIndex; - t->heapIndex = 0; + t->m->heapPool[t->m->heapPoolIndex++] = t->heap; + t->heapOffset += t->heapIndex; + t->heapIndex = 0; + } } } + break; + + case Machine::FixedAllocation: + if (t->m->fixedFootprint + sizeInBytes > FixedFootprintThresholdInBytes) + { + t->heap = 0; + } + break; + + case Machine::ImmortalAllocation: + break; } - break; - case Machine::FixedAllocation: - if (t->m->fixedFootprint + sizeInBytes > FixedFootprintThresholdInBytes) { - t->heap = 0; + if (t->heap == 0) { + // fprintf(stderr, "gc"); + // vmPrintTrace(t); + collect(t, Heap::MinorCollection); } - break; - - case Machine::ImmortalAllocation: - break; - } - - if (t->heap == 0) { -// fprintf(stderr, "gc"); -// vmPrintTrace(t); - collect(t, Heap::MinorCollection); - } + } while (type == Machine::MovableAllocation + and t->heapIndex + ceiling(sizeInBytes, BytesPerWord) + > ThreadHeapSizeInWords); switch (type) { case Machine::MovableAllocation: { diff --git a/src/machine.h b/src/machine.h index 942d92dd57..c75626c5e9 100644 --- a/src/machine.h +++ b/src/machine.h @@ -1519,6 +1519,9 @@ allocate3(Thread* t, Allocator* allocator, Machine::AllocationType type, inline object allocateSmall(Thread* t, unsigned sizeInBytes) { + assert(t, t->heapIndex + ceiling(sizeInBytes, BytesPerWord) + <= ThreadHeapSizeInWords); + object o = reinterpret_cast(t->heap + t->heapIndex); t->heapIndex += ceiling(sizeInBytes, BytesPerWord); cast(o, 0) = 0;