Two-line change: add #ifdef MMGC_THREADSAFE in one more place
authorjorendorff@mozilla.com
Fri, 30 Nov 2007 16:12:38 -0600
changeset 14 d0f7c6e7ba23ae43885dcb69befe8a44854f8ab2
parent 12 af07ce9c5d6110146c21f1fa02909c89a644885b
child 15 208dbe8b1541075ecdd53987d78ce4fa271669dd
push id1
push userbsmedberg@mozilla.com
push dateMon, 21 Apr 2008 01:54:18 +0000
Two-line change: add #ifdef MMGC_THREADSAFE in one more place
mmgc-threadsafe-take2
--- a/mmgc-threadsafe-take2
+++ b/mmgc-threadsafe-take2
@@ -272,46 +272,49 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 +		{
 +			USING_CALLBACK_LIST(this);
 +			for (GCCallback *cb = m_callbacks; cb; cb = cb->nextCB)
 +				cb->lastmark(work);
 +		}
  
  		if(stackStart == NULL) {
  			MarkQueueAndStack(work);
-@@ -476,6 +661,17 @@ namespace MMgc
+@@ -476,6 +661,20 @@ namespace MMgc
  
  	void *GC::Alloc(size_t size, int flags/*0*/, int skip/*3*/)
  	{
 +#ifdef MMGC_THREADSAFE
 +		GCAutoLock _lock(m_lock);
 +		GCAssert(!m_gcRunning);
 +#else
 +		CheckThread();
 +#endif
 +		return AllocAlreadyLocked(size, flags, skip);
 +	}
 +
 +	void *GC::AllocAlreadyLocked(size_t size, int flags/*0*/, int skip/*3*/)
 +	{
++#ifdef MMGC_THREADSAFE
++		GCAssert(GCThread::GetCurrentThread()->IsInActiveRequest());
++#endif
  #ifdef DEBUGGER
  		avmplus::AvmCore *core = (avmplus::AvmCore*)GetGCContextVariable(GCV_AVMCORE);
  		if(core)
-@@ -487,9 +683,8 @@ namespace MMgc
+@@ -487,9 +686,8 @@ namespace MMgc
  
  #ifdef _DEBUG
  		GCAssert(size + 7 > size);
 -		CheckThread();
  		if (GC::greedy) {
 -			Collect();
 +			CollectWithBookkeeping(true, true);
  		}
  		// always be marking in pedantic mode
  		if(incrementalValidationPedantic) {
-@@ -572,15 +767,21 @@ namespace MMgc
+@@ -572,15 +770,21 @@ namespace MMgc
  
  	void GC::Free(void *item)
  	{
 +#ifdef MMGC_THREADSAFE
 +		GCAutoLock _lock(m_lock);
 +		GCAssert(!m_gcRunning);
 +#else
  		CheckThread();
@@ -329,46 +332,46 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 -			return;
 -		}
 -
 -		bool isLarge;
 -
  		if(collecting) {
  			goto bail;
  		}
-@@ -629,6 +830,8 @@ bail:
+@@ -629,6 +833,8 @@ bail:
  
  	void GC::ClearMarks()
  	{
 +		ASSERT_EXCLUSIVE_GC();
 +
  		for (int i=0; i < kNumSizeClasses; i++) {
  #ifdef MMGC_DRC
  			containsPointersRCAllocs[i]->ClearMarks();
-@@ -641,6 +844,8 @@ bail:
+@@ -641,6 +847,8 @@ bail:
  
  	void GC::Finalize()
  	{
 +		ASSERT_EXCLUSIVE_GC();
 +
  		for(int i=kNumSizeClasses; i-- > 0;) {
  #ifdef MMGC_DRC
  			containsPointersRCAllocs[i]->Finalize();
-@@ -662,7 +867,9 @@ bail:
+@@ -662,7 +870,9 @@ bail:
  	}
  
  	void GC::Sweep(bool force)
 -	{	
 +	{
 +		ASSERT_EXCLUSIVE_GC();
 +
  		SAMPLE_FRAME("[sweep]", core());
  		sweeps++;
  
-@@ -681,10 +888,13 @@ bail:
+@@ -681,10 +891,13 @@ bail:
  		collecting = true;
  
  		// invoke presweep on all callbacks
 -		GCCallback *cb = m_callbacks;
 -		while(cb) {
 -			cb->presweep();
 -			cb = cb->nextCB;
 +		{
@@ -376,17 +379,17 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 +			GCCallback *cb = m_callbacks;
 +			while(cb) {
 +				cb->presweep();
 +				cb = cb->nextCB;
 +			}
  		}
  
  		SAMPLE_CHECK();
-@@ -737,12 +947,15 @@ bail:
+@@ -737,12 +950,15 @@ bail:
  		collecting = false;
  
  		// invoke postsweep callback
 -		cb = m_callbacks;
 -		while(cb) {
 -			cb->postsweep();
 -			cb = cb->nextCB;
 -		}
@@ -398,129 +401,129 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 +				cb->postsweep();
 +				cb = cb->nextCB;
 +			}
 +		}
 +
  		SAMPLE_CHECK();
  
  		allocsSinceCollect = 0;
-@@ -779,6 +992,7 @@ bail:
+@@ -779,6 +995,7 @@ bail:
  
  	void* GC::AllocBlock(int size, int pageType, bool zero)
  	{
 +		ASSERT_GC_LOCK();
  #ifdef DEBUGGER
  		AllocActivity(size);
  #endif
-@@ -796,7 +1010,7 @@ bail:
+@@ -796,7 +1013,7 @@ bail:
  			if(incremental)
  				StartIncrementalMark();
  			else
 -				Collect();
 +				CollectWithBookkeeping(true, true);
  		}
  
  		void *item;
-@@ -837,6 +1051,8 @@ bail:
+@@ -837,6 +1054,8 @@ bail:
  
  	void* GC::AllocBlockIncremental(int size, bool zero)
  	{
 +		ASSERT_GC_LOCK();
 +
  		if(!nogc && !collecting) {
  			uint64 now = GetPerformanceCounter();
  			if (marking) {		
-@@ -864,12 +1080,14 @@ bail:
+@@ -864,12 +1083,14 @@ bail:
  
  	void* GC::AllocBlockNonIncremental(int size, bool zero)
  	{
 +		ASSERT_GC_LOCK();
 +
  		void *item = heap->Alloc(size, false, zero);
  		if (!item) {
  			if (heap->GetTotalHeapSize() >= collectThreshold &&
  				allocsSinceCollect >= totalGCPages / kFreeSpaceDivisor) 
  			{
 -				Collect();
 +				CollectWithBookkeeping(true, true);
  				item = heap->Alloc(size, false, zero);
  			}
  		}
-@@ -878,6 +1096,11 @@ bail:
+@@ -878,6 +1099,11 @@ bail:
  
  	void GC::FreeBlock(void *ptr, uint32 size)
  	{
 +#ifdef MMGC_THREADSAFE
 +		GCAssert(m_lock.IsHeld() || destroying
 +				 || (m_gcRunning
 +					 && m_exclusiveGCThread == GCThread::GetCurrentThread()));
 +#endif
  #ifdef DEBUGGER
  		AllocActivity(- (int)size);
  #endif
-@@ -905,6 +1128,7 @@ bail:
+@@ -905,6 +1131,7 @@ bail:
  
  	void GC::Mark(GCStack<GCWorkItem> &work)
  	{
 +		ASSERT_EXCLUSIVE_GC();
  		while(work.Count()) {
  			MarkItem(work);
  		}
-@@ -912,6 +1136,7 @@ bail:
+@@ -912,6 +1139,7 @@ bail:
  
  	void GC::MarkGCPages(void *item, uint32 numPages, int to)
  	{
 +		USING_PAGE_MAP();
  		uintptr addr = (uintptr)item;
  		uint32 shiftAmount=0;
  		unsigned char *dst = pageMap;
-@@ -954,7 +1179,7 @@ bail:
+@@ -954,7 +1182,7 @@ bail:
  		addr = (uintptr)item;
  		while(numPages--)
  		{
 -			GCAssert(GetPageMapValue(addr) == 0);
 +			GCAssert(GetPageMapValueAlreadyLocked(addr) == 0);
  			SetPageMapValue(addr, to);
  			addr += GCHeap::kBlockSize;
  		}
-@@ -963,6 +1188,8 @@ bail:
+@@ -963,6 +1191,8 @@ bail:
  	void GC::UnmarkGCPages(void *item, uint32 numpages)
  	{
  		uintptr addr = (uintptr) item;
 +
 +		USING_PAGE_MAP();
  		while(numpages--)
  		{
  			ClearPageMapValue(addr);
-@@ -1128,6 +1355,7 @@ bail:
+@@ -1128,6 +1358,7 @@ bail:
  		gc->Mark(work);
  	}
  
 +#ifndef MMGC_THREADSAFE
  	void GC::CheckThread()
  	{
  #ifdef _DEBUG
-@@ -1136,12 +1364,13 @@ bail:
+@@ -1136,12 +1367,13 @@ bail:
  #endif
  #endif
  	}
 -
 +#endif
  
  	bool GC::IsPointerToGCPage(const void *item)
  	{
 +		USING_PAGE_MAP();
  		if((uintptr)item >= memStart && (uintptr)item < memEnd)
 -			return GetPageMapValue((uintptr) item) != 0;
 +			return GetPageMapValueAlreadyLocked((uintptr) item) != 0;
  		return false;
  	}
  
-@@ -1182,10 +1411,13 @@ bail:
+@@ -1182,10 +1414,13 @@ bail:
  		// note if we add things while reaping we could delete the object
  		// here if we had a way to monitor our stack usage
  		if(reaping && PLENTY_OF_STACK()) {
 -			GCCallback *cb = gc->m_callbacks;
 -			while(cb) {
 -				cb->prereap(obj);
 -				cb = cb->nextCB;
 +			{
@@ -528,17 +531,17 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 +				GCCallback *cb = gc->m_callbacks;
 +				while(cb) {
 +					cb->prereap(obj);
 +					cb = cb->nextCB;
 +				}
  			}
  			if(gc->IsFinalized(obj))
  				((GCFinalizable*)obj)->~GCFinalizable();
-@@ -1376,10 +1608,13 @@ bail:
+@@ -1376,10 +1611,13 @@ bail:
  		nextPinnedIndex = 0;
  
  		// invoke prereap on all callbacks
 -		GCCallback *cb = gc->m_callbacks;
 -		while(cb) {
 -			cb->prereap();
 -			cb = cb->nextCB;
 +		{
@@ -546,17 +549,17 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 +			GCCallback *cb = gc->m_callbacks;
 +			while(cb) {
 +				cb->prereap();
 +				cb = cb->nextCB;
 +			}
  		}
  
  #ifdef _DEBUG
-@@ -1420,10 +1655,13 @@ bail:
+@@ -1420,10 +1658,13 @@ bail:
  				}
  #endif
  				// invoke prereap on all callbacks
 -				GCCallback *cbb = gc->m_callbacks;
 -				while(cbb) {
 -					cbb->prereap(rcobj);
 -					cbb = cbb->nextCB;
 +				{
@@ -564,17 +567,17 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 +					GCCallback *cbb = gc->m_callbacks;
 +					while(cbb) {
 +						cbb->prereap(rcobj);
 +						cbb = cbb->nextCB;
 +					}
  				}
  
  				GCAssert(*(int*)rcobj != 0);
-@@ -1457,10 +1695,13 @@ bail:
+@@ -1457,10 +1698,13 @@ bail:
  		zctIndex = nextPinnedIndex = 0;
  
  		// invoke postreap on all callbacks
 -		cb = gc->m_callbacks;
 -		while(cb) {
 -			cb->postreap();
 -			cb = cb->nextCB;
 +		{
@@ -582,27 +585,27 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 +			GCCallback *cb = gc->m_callbacks;
 +			while(cb) {
 +				cb->postreap();
 +				cb = cb->nextCB;
 +			}
  		}
  #ifdef DEBUGGER
  		if(gc->gcstats && numObjects) {
-@@ -1607,7 +1848,8 @@ bail:
+@@ -1607,7 +1851,8 @@ bail:
  		va_end(argptr);
  
  		GCAssert(strlen(buf) < 4096);
 -			
 +
 +		USING_CALLBACK_LIST(this);
  		GCCallback *cb = m_callbacks;
  		while(cb) {
  			cb->log(buf);
-@@ -1659,23 +1901,27 @@ bail:
+@@ -1659,23 +1904,27 @@ bail:
  
  	bool GC::IsRCObject(const void *item)
  	{
 -		if((uintptr)item >= memStart && (uintptr)item < memEnd && ((uintptr)item&0xfff) != 0)
 -		{
 -			int bits = GetPageMapValue((uintptr)item);		
 -			item = GetRealPointer(item);
 -			switch(bits)
@@ -636,169 +639,169 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 +		case kGCLargeAllocPageFirst:
 +			return GCLargeAlloc::IsRCObject(item);
 +		default:
 +			return false;
 +		}
  	}
  
  #endif // MMGC_DRC
-@@ -1761,7 +2007,7 @@ bail:
+@@ -1761,7 +2010,7 @@ bail:
  				continue;
  			
  			// normalize and divide by 4K to get index
 -			int bits = GetPageMapValue(val);
 +			int bits = GetPageMapValueAlreadyLocked(val);
  			switch(bits)
  			{
  			case 0:
-@@ -1782,6 +2028,8 @@ bail:
+@@ -1782,6 +2031,8 @@ bail:
  
  	void GC::FindUnmarkedPointers()
  	{
 +		ASSERT_EXCLUSIVE_GC();
 +
  		if(findUnmarkedPointers)
  		{
  			uintptr m = memStart;
-@@ -1920,6 +2168,11 @@ bail:
+@@ -1920,6 +2171,11 @@ bail:
  	 */
  	void GC::WhosPointingAtMe(void* me, int recurseDepth, int currentDepth)
  	{
 +#ifdef MMGC_THREADSAFE
 +		if (currentDepth == 0)
 +			pageMapLock.Acquire();
 +#endif
 +
  		uintptr val = (uintptr)me;
  		uintptr m = memStart;
  
-@@ -1942,7 +2195,7 @@ bail:
+@@ -1942,7 +2198,7 @@ bail:
  #endif
  
  			// divide by 4K to get index
 -			int bits = GetPageMapValue(m);
 +			int bits = GetPageMapValueAlreadyLocked(m);
  			if(bits == kNonGC) 
  			{
  				ProbeForMatch((const void*)m, GCHeap::kBlockSize, val, recurseDepth, currentDepth);
-@@ -1986,6 +2239,11 @@ bail:
+@@ -1986,6 +2242,11 @@ bail:
  			
  			}
  		}
 +
 +#ifdef MMGC_THREADSAFE
 +		if (currentDepth == 0)
 +			pageMapLock.Release();
 +#endif
  	}
  #undef ALLOCA_AND_FILL_WITH_SPACES
  #endif
-@@ -2389,9 +2647,11 @@ bail:
+@@ -2389,9 +2650,11 @@ bail:
  		return 1000000;
  		#endif
  	}
 -	
 +
  	void GC::IncrementalMark(uint32 time)
  	{
 +		ASSERT_EXCLUSIVE_GC();
 +
  		SAMPLE_FRAME("[mark]", core());
  		if(m_incrementalWork.Count() == 0 || hitZeroObjects) {
  			FinishIncrementalMark();
-@@ -2446,6 +2706,8 @@ bail:
+@@ -2446,6 +2709,8 @@ bail:
  
  	void GC::FinishIncrementalMark()
  	{
 +		ASSERT_EXCLUSIVE_GC();
 +
  		// Don't finish an incremental mark (i.e., sweep) if we
  		// are in the midst of a ZCT reap.
  		if (zct.reaping)
-@@ -2477,8 +2739,11 @@ bail:
+@@ -2477,8 +2742,11 @@ bail:
  		}
  
  		// invoke lastmark on all callbacks
 -		for (GCCallback *cb = m_callbacks; cb; cb = cb->nextCB)
 -			cb->lastmark(m_incrementalWork);
 +		{
 +			USING_CALLBACK_LIST(this);
 +			for (GCCallback *cb = m_callbacks; cb; cb = cb->nextCB)
 +				cb->lastmark(m_incrementalWork);
 +		}
  
  		MarkQueueAndStack(m_incrementalWork);
  
-@@ -2629,6 +2894,7 @@ bail:
+@@ -2629,6 +2897,7 @@ bail:
  	{
  		uint32 *bits;
  
 +		ASSERT_GC_LOCK();
  		GCAssert(numBytes % 4 == 0);
  
  		#ifdef MMGC_64BIT // we use first 8-byte slot for the free list
-@@ -2700,7 +2966,8 @@ bail:
+@@ -2700,7 +2969,8 @@ bail:
  
  	void GC::AddCallback(GCCallback *cb)
  	{
 -		CheckThread();
 +		USING_CALLBACK_LIST(this);
 +
  		cb->prevCB = NULL;
  		cb->nextCB = m_callbacks;
  		if(m_callbacks)
-@@ -2710,7 +2977,8 @@ bail:
+@@ -2710,7 +2980,8 @@ bail:
  
  	void GC::RemoveCallback(GCCallback *cb)
  	{
 -		CheckThread();
 +		USING_CALLBACK_LIST(this);
 +
  		if( m_callbacks == cb )
  			m_callbacks = cb->nextCB;
  		else
-@@ -2722,7 +2990,8 @@ bail:
+@@ -2722,7 +2993,8 @@ bail:
  
  	void GC::AddEdgeCallback(GCEdgeCallback *cb)
  	{
 -		CheckThread();
 +		USING_CALLBACK_LIST(this);
 +
  		cb->prevCB = NULL;
  		cb->nextCB = m_edgeCallbacks;
  		if(m_edgeCallbacks)
-@@ -2732,7 +3001,8 @@ bail:
+@@ -2732,7 +3004,8 @@ bail:
  
  	void GC::RemoveEdgeCallback(GCEdgeCallback *cb)
  	{
 -		CheckThread();
 +		USING_CALLBACK_LIST(this);
 +
  		if( m_edgeCallbacks == cb )
  			m_edgeCallbacks = cb->nextCB;
  		else
-@@ -2744,12 +3014,12 @@ bail:
+@@ -2744,12 +3017,12 @@ bail:
  
  	void GC::FireFoundEdgeTo(const void *p)
  	{
 +		// Don't acquire the spinlock here because (a) that would really hurt
 +		// performance; (b) the m_edgeCallbacks list, unlike the m_callbacks
 +		// list, is protected by the request model.
  		p = GetUserPointer(p);
 -		GCEdgeCallback *cb = m_edgeCallbacks;
 -		while(cb) {
 +		for (GCEdgeCallback *cb = m_edgeCallbacks; cb; cb = cb->nextCB)
  			cb->foundEdgeTo(p);
 -			cb = cb->nextCB;
 -		}
  	}
  
  	void GC::PushWorkItem(GCStack<GCWorkItem> &stack, GCWorkItem item)
-@@ -2762,9 +3032,18 @@ bail:
+@@ -2762,9 +3035,18 @@ bail:
  	GCWeakRef* GC::GetWeakRef(const void *item) 
  	{
  		GC *gc = GetGC(item);
 +#ifdef MMGC_THREADSAFE
 +		GCAutoLock _lock(gc->m_lock);
 +#endif
  		GCWeakRef *ref = (GCWeakRef*) gc->weakRefs.get(item);
 +
@@ -807,42 +810,42 @@ diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
 +			void *p = gc->AllocAlreadyLocked(sizeof(GCWeakRef), GC::kFinalize, 4);
 +			ref = new (p) GCWeakRef(item);
 +#else
  			ref = new (gc) GCWeakRef(item);
 +#endif
  			gc->weakRefs.put(item, ref);
  			item = GetRealPointer(item);
  			if (GCLargeAlloc::IsLargeBlock(item)) {
-@@ -2824,6 +3103,8 @@ bail:
+@@ -2824,6 +3106,8 @@ bail:
  
  	void GC::FindMissingWriteBarriers()
  	{
 +		ASSERT_EXCLUSIVE_GC();
 +
  		if(!incrementalValidation)
  			return;
  
-@@ -2885,6 +3166,7 @@ bail:
+@@ -2885,6 +3169,7 @@ bail:
  	void GC::StartGCActivity()
  	{
  		// invoke postsweep callback
 +		USING_CALLBACK_LIST(this);
  		GCCallback *cb = m_callbacks;
  		while(cb) {
  			cb->startGCActivity();
-@@ -2895,6 +3177,7 @@ bail:
+@@ -2895,6 +3180,7 @@ bail:
  	void GC::StopGCActivity()
  	{
  		// invoke postsweep callback
 +		USING_CALLBACK_LIST(this);
  		GCCallback *cb = m_callbacks;
  		while(cb) {
  			cb->stopGCActivity();
-@@ -2905,6 +3188,7 @@ bail:
+@@ -2905,6 +3191,7 @@ bail:
  	void GC::AllocActivity(int blocks)
  	{
  		// invoke postsweep callback
 +		USING_CALLBACK_LIST(this);
  		GCCallback *cb = m_callbacks;
  		while(cb) {
  			cb->allocActivity(blocks);
 diff --git a/MMgc/GC.h b/MMgc/GC.h