tamarin-bug-427030-v1.patch
author Benjamin Smedberg <benjamin@smedbergs.us>
Sat, 26 Jul 2008 22:49:39 -0400
changeset 167 a4da40849f5436e629c5732f4368c6c48189637f
parent 154 7b66eb66ba95f04cfa2392550b012262466a1275
permissions -rw-r--r--
State as of now

* * *

diff --git a/js/tamarin/MMgc/GC.cpp b/js/tamarin/MMgc/GC.cpp
--- a/js/tamarin/MMgc/GC.cpp
+++ b/js/tamarin/MMgc/GC.cpp
@@ -629,6 +629,7 @@ namespace MMgc
 			PushWorkItem(work, item);
 			Mark(work);
 		}
+		MarkLinks(work);
 		
 		m_currentQueue = NULL;
 
@@ -838,6 +839,14 @@ namespace MMgc
 		} else {
 			GCAlloc::Free(GetRealPointer(item));
 		}
+
+		// Remove all links leading from item.
+		{
+			HardLinks::iterator i =
+				m_hardLinks.lower_bound(PtrPair(item, NULL));  // is NULL guaranteed to be less than all other pointers?
+			while (i != m_hardLinks.end() && i->first.first == item)
+				m_hardLinks.erase(i++);
+		}
 		return;
 
 bail:
@@ -862,6 +871,18 @@ bail:
 			noPointersAllocs[i]->ClearMarks();
 		}
 		largeAlloc->ClearMarks();
+	}
+
+	void GC::SweepLinks()
+	{
+		HardLinks::iterator i = m_hardLinks.begin();
+		while (i != m_hardLinks.end()) {
+			const void *source = i->first.first;
+			if (!GetMark(source))
+				m_hardLinks.erase(i++);
+			else
+				++i;
+		}
 	}
 
 	void GC::Finalize()
@@ -908,7 +929,7 @@ bail:
 			DumpMemoryInfo();
 		}
 #endif
-		
+		SweepLinks();
 
 		// invoke presweep on all callbacks
 		for (GCCallback *cb = m_callbacks; cb; cb = cb->nextCB)
@@ -1292,6 +1313,29 @@ bail:
 
 		PushWorkItem(work, item);
 		Mark(work);
+	}
+
+	void GC::MarkLinks(GCStack<GCWorkItem> &work)
+	{
+		for (;;) {
+			bool found = false;
+			for (HardLinks::iterator i = m_hardLinks.begin();
+			     i != m_hardLinks.end();
+			     ++i) {
+				const void *source = i->first.first;
+				const void *target = i->first.second;
+
+				GCAssert(i->second != 0);
+				if (GetMark(source) && !GetMark(target)) {
+					GCWorkItem item(target, Size(target), true);
+					PushWorkItem(work, item);
+					found = true;
+				}
+			}
+			if (!found)
+				break;
+			Mark(work);
+		}
 	}
 
 	GCRoot::GCRoot(GC * _gc) : gc(_gc)
@@ -2758,6 +2802,7 @@ bail:
 			cb->lastmark(m_incrementalWork);
 
 		MarkQueueAndStack(m_incrementalWork);
+		MarkLinks(m_incrementalWork);
 
 		m_currentQueue = NULL;
 
@@ -2975,6 +3020,43 @@ bail:
 			root->next->prev = root->prev;
 	}
 
+	void GC::Link(void *source, void *target)
+	{
+#ifdef MMGC_THREADSAFE
+		GCAutoLock _lock(m_lock);
+		GCAssert(!m_gcRunning);
+#else
+		CheckThread();
+#endif
+		GCAssert(source != NULL);
+		GCAssert(IsGCMemory(source));
+		GCAssert(source == FindBeginning(source));
+		GCAssert(ContainsPointers(source));
+		GCAssert(target != NULL);
+		GCAssert(IsGCMemory(target));
+		GCAssert(target == FindBeginning(target));
+
+		size_t i = ++m_hardLinks[PtrPair(source, target)];
+		(void) i;
+		GCAssert(i != 0);
+	}
+
+	void GC::Unlink(void *source, void *target)
+	{
+#ifdef MMGC_THREADSAFE
+		GCAutoLock _lock(m_lock);
+		GCAssert(!m_gcRunning);
+#else
+		CheckThread();
+#endif
+
+		PtrPair pair(source, target);
+		HardLinks::iterator iter = m_hardLinks.find(pair);
+		GCAssert(iter != m_hardLinks.end());
+		if (--iter->second == 0)
+			m_hardLinks.erase(iter);
+	}
+
 	void GC::AddCallback(GCCallback *cb)
 	{
 		cb->prevCB = NULL;
diff --git a/js/tamarin/MMgc/GC.h b/js/tamarin/MMgc/GC.h
--- a/js/tamarin/MMgc/GC.h
+++ b/js/tamarin/MMgc/GC.h
@@ -39,6 +39,9 @@
 
 #ifndef __GC__
 #define __GC__
+
+#include <map>
+#include <utility>
 
 #if defined MMGC_IA32
 
@@ -733,6 +736,12 @@ namespace MMgc
 		 */
 		void Free(const void *ptr);
 
+		/** @access Requires(request) */
+		void Link(void *source, void *target);
+
+		/** @access Requires(request) */
+		void Unlink(void *source, void *target);
+
 		/**
 		 * return the size of a piece of memory, may be bigger than what was asked for
 		 *
@@ -1365,9 +1374,13 @@ namespace MMgc
 		/** @access Requires(exclusiveGC) */
 		void ForceSweep() { Sweep(true); }
 		/** @access Requires(exclusiveGC) */
+		void SweepLinks();
+		/** @access Requires(exclusiveGC) */
 		void Mark(GCStack<GCWorkItem> &work);
 		/** @access Requires(exclusiveGC) */
 		void MarkQueueAndStack(GCStack<GCWorkItem> &work);
+		/** @access Requires(exclusiveGC) */
+		void MarkLinks(GCStack<GCWorkItem> &work);
 		/** @access Requires(exclusiveGC) */
 		void MarkItem(GCStack<GCWorkItem> &work)
 		{
@@ -1442,7 +1455,12 @@ namespace MMgc
 		GCRoot *m_roots;
 		void AddRoot(GCRoot *root);
 		void RemoveRoot(GCRoot *root);
-		
+
+		typedef std::pair<const void *, const void *> PtrPair;
+		typedef std::map<PtrPair, size_t> HardLinks;
+		/** @access Requires(request && m_lock || exclusiveGC) */
+		HardLinks m_hardLinks;
+
 		/**
 		 * Points to the head of a linked list of callback objects.
 		 *
diff --git a/js/tamarin/MMgc/GCTests.cpp b/js/tamarin/MMgc/GCTests.cpp
--- a/js/tamarin/MMgc/GCTests.cpp
+++ b/js/tamarin/MMgc/GCTests.cpp
@@ -173,6 +173,96 @@ namespace MMgc
 			delete wr->get();
 	}
 
+	class HardLinkTests {
+		static GCObject *a;
+		static GCObject *b;
+		static GCWeakRef *wra;
+		static GCWeakRef *wrb;
+
+		template <class Fn>
+		static void _tidy2(Fn fn)
+		{
+			char buf[64];
+			sprintf(buf, "%p", buf);  // don't optimize away buf
+			fn();
+		}
+
+		// Call a function and wipe its prints off the stack afterwards.
+		template <class Fn>
+		static void tidy(Fn fn)
+		{
+			_tidy2(fn);
+			gc->CleanStack(true);
+		}
+
+		static void makeObjects(bool linkAB, bool linkBA) {
+			a = new(gc) GCObject;
+			b = new(gc) GCObject;
+			if (linkAB) gc->Link(a, b);
+			if (linkBA) gc->Link(b, a);
+			wra = a->GetWeakRef();
+			wrb = b->GetWeakRef();
+		}
+
+		static void makeNonLinkedObjects() { makeObjects(false, false); }
+		static void makeLinkedObjects() { makeObjects(true, false); }
+		static void makeLinkCycle() { makeObjects(true, true); }
+
+		// Test 0: make A and B, then make A unreachable.
+		// Check that only A is collected.
+		static void test0() {
+			tidy(makeNonLinkedObjects);
+			volatile void *keep[] = {b, wra, wrb};  (void) keep;
+			collect();
+			GCAssert(wra->get() == NULL);
+			GCAssert(wrb->get() != NULL);
+		}
+
+		// Test 1: link A->B, then make B unreachable.
+		// Check that neither is collected.
+		static void test1() {
+			tidy(makeLinkedObjects);
+			volatile void *keep[] = {a, wra, wrb};  (void) keep;
+			collect();
+			GCAssert(wra->get() != NULL);
+			GCAssert(wrb->get() != NULL);
+		}
+
+		// Test 2: link A->B, then make A unreachable.
+		// Check that A is collected.
+		static void test2() {
+			tidy(makeLinkedObjects);
+			volatile void *keep[] = {b, wra, wrb};  (void) keep;
+			collect();
+			GCAssert(wra->get() == NULL);
+			GCAssert(wrb->get() != NULL);
+		}
+
+		// Test 3: link A->B, B->A, then make both unreachable.
+		// Check that both are collected.
+		static void test3() {
+			tidy(makeLinkCycle);
+			volatile void *keep[] = {wra, wrb};  (void) keep;
+			collect();
+			GCAssert(wra->get() == NULL);
+			GCAssert(wrb->get() == NULL);
+		}
+
+	public:
+		static void run()
+		{
+			tidy(test0);
+			tidy(test1);
+			tidy(test2);
+			tidy(test3);
+		}
+	};
+
+	GCObject *HardLinkTests::a;
+	GCObject *HardLinkTests::b;
+	GCWeakRef *HardLinkTests::wra;
+	GCWeakRef *HardLinkTests::wrb;
+
 	void RunGCTests(GC *g)
 	{
 		gc = g;
@@ -184,6 +274,7 @@ namespace MMgc
 		weakRefFreeSmall();
 		weakRefFreeLarge();
 		drcApolloTest();
+		HardLinkTests::run();
 #ifdef MMGC_THREADSAFE
 		g->OnLeaveRequest();
 #endif