File 0001-Use-dynamic-allocation-for-plugin-class-in... of Package compiz

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-Use-dynamic-allocation-for-plugin-class-indexes-and-.patch of Package compiz (Revision 9a0c2cc6ccc7181ec639fbd1710d3f12)

Currently displaying revision 9a0c2cc6ccc7181ec639fbd1710d3f12 , Show latest

From 5d418445e7b14719debd5397db8ddb7a506af9b0 Mon Sep 17 00:00:00 2001
From: Sam Spilsbury <smspillaz@gmail.com>
Date: Wed, 30 Jun 2010 07:34:47 +0800
Subject: [PATCH] Use dynamic allocation for plugin class indexes and store them in screen's CompPrivate's

Previously we used a static variable in class templates to do reference counting of each unique formation of a class. However, there is a bug in gcc or ld which can
cause the variable's contructor to be called in dlopen of another plugin with a similar template with static variables. This in turn generally causes mayhem as the
internal reference count goes to zero, and when a window is next closed, all plugin indexes are free'd even though there are a number of classes that still need
them to be open. And then PluginClassHandler::get () can return NULL, which should never happen (since plugin classes are not NULL checked).

This should fix bugs confirmed to happen with Arch Linux, OpenSUSE 11.3 and any other distribution that uses the GCC 4.5 prerelease.
---
 include/core/pluginclasshandler.h |  126 +++++++++++++++++++++++++------------
 1 files changed, 86 insertions(+), 40 deletions(-)

diff --git a/include/core/pluginclasshandler.h b/include/core/pluginclasshandler.h
index dde9b0d..48bcbf1 100644
--- a/include/core/pluginclasshandler.h
+++ b/include/core/pluginclasshandler.h
@@ -56,35 +56,47 @@ class PluginClassHandler {
     private:
 	bool mFailed;
 	Tb   *mBase;
-
-	static PluginClassIndex mIndex;
 };
 
 template<class Tp, class Tb, int ABI>
-PluginClassIndex PluginClassHandler<Tp,Tb,ABI>::mIndex;
-
-template<class Tp, class Tb, int ABI>
 PluginClassHandler<Tp,Tb,ABI>::PluginClassHandler (Tb *base) :
     mFailed (false),
     mBase (base)
 {
-    if (mIndex.pcFailed)
+    /* Store our index reference count as a value in screen */
+    CompString  indexKeyName = compPrintf ("%s_index_%lu_structure", typeid (Tp).name (), ABI);
+    CompPrivate pIndex;
+    PluginClassIndex *index;
+    
+    if (screen->hasValue (indexKeyName))
     {
-	mFailed = true;
+	pIndex = screen->getValue (indexKeyName);
+	index = (PluginClassIndex *) pIndex.ptr;
     }
     else
     {
-	if (!mIndex.initiated)
+	index = new PluginClassIndex;
+	pIndex.ptr = (void *) index;
+	screen->storeValue (indexKeyName, pIndex);
+    }
+    
+    if (index && index->failed)
+    {
+	mFailed = true;
+    }
+    else if (index)
+    {
+	if (!index->initiated)
 	{
-	    mIndex.index = Tb::allocPluginClassIndex ();
-	    if (mIndex.index != (unsigned)~0)
+	    index->index = Tb::allocPluginClassIndex ();
+	    if (index->index != (unsigned)~0)
 	    {
-		mIndex.initiated = true;
-		mIndex.failed    = false;
-		mIndex.pcIndex = pluginClassHandlerIndex;
+		index->initiated = true;
+		index->failed    = false;
+		index->pcIndex = pluginClassHandlerIndex;
 
 		CompPrivate p;
-		p.uval = mIndex.index;
+		p.uval = index->index;
 
 		if (!screen->hasValue (keyName ()))
 		{
@@ -100,36 +112,62 @@ PluginClassHandler<Tp,Tb,ABI>::PluginClassHandler (Tb *base) :
 	    }
 	    else
 	    {
-		mIndex.failed = true;
-		mIndex.initiated = false;
-		mIndex.pcFailed = true;
-		mIndex.pcIndex = pluginClassHandlerIndex;
+		index->failed = true;
+		index->initiated = false;
+		index->pcFailed = true;
+		index->pcIndex = pluginClassHandlerIndex;
 		mFailed = true;
 	    }
 	}
 
-	if (!mIndex.failed)
+	if (!index->failed)
 	{
-	    mIndex.refCount++;
-	    mBase->pluginClasses[mIndex.index] = static_cast<Tp *> (this);
+	    mBase->pluginClasses[index->index] = static_cast<Tp *> (this);
 	}
     }
+    else
+    {
+	compLogMessage ("core", CompLogLevelFatal,
+	    "Index allocation for \"%s\" failed. Possibly out of memory?",
+	    keyName ().c_str ());
+    }
 }
 
 template<class Tp, class Tb, int ABI>
 PluginClassHandler<Tp,Tb,ABI>::~PluginClassHandler ()
 {
-    if (!mIndex.pcFailed)
+    CompString       indexKeyName = compPrintf ("%s_index_%lu_structure", typeid (Tp).name (), ABI);
+    CompPrivate      pIndex = screen->getValue (indexKeyName);
+    PluginClassIndex *index = (PluginClassIndex *) pIndex.ptr;
+
+    if (index && !index->pcFailed)
     {
-	mIndex.refCount--;
+	/* Store our index reference count as a value in screen */
+	CompString  refCountKeyName = compPrintf ("%s_index_%lu_refCnt", typeid (Tp).name (), ABI);
+	CompPrivate pIndexRefCount;
+	
+	if (screen->hasValue (refCountKeyName))
+	{
+	    pIndexRefCount = screen->getValue (refCountKeyName);
+	    screen->eraseValue (refCountKeyName);
+	}
+	else
+	{
+	    pIndexRefCount.uval = 0;
+	}
+	
+	pIndexRefCount.uval--;
+	screen->storeValue (refCountKeyName, pIndexRefCount);
 
-	if (mIndex.refCount == 0)
+	if (pIndexRefCount.uval == 0)
 	{
-	    Tb::freePluginClassIndex (mIndex.index);
-	    mIndex.initiated = false;
-	    mIndex.failed = false;
-	    mIndex.pcIndex = pluginClassHandlerIndex;
+	    Tb::freePluginClassIndex (index->index);
+	    index->initiated = false;
+	    index->failed = false;
+	    index->pcIndex = pluginClassHandlerIndex;
 	    screen->eraseValue (keyName ());
+	    delete index;
+	    screen->eraseValue (indexKeyName);
 	    pluginClassHandlerIndex++;
 	}
     }
@@ -139,27 +177,35 @@ template<class Tp, class Tb, int ABI>
 Tp *
 PluginClassHandler<Tp,Tb,ABI>::get (Tb *base)
 {
-    if (mIndex.initiated && pluginClassHandlerIndex == mIndex.pcIndex)
+    CompString       indexKeyName = compPrintf ("%s_index_%lu_structure", typeid (Tp).name (), ABI);
+    CompPrivate      pIndex = screen->getValue (indexKeyName);
+    PluginClassIndex *index = (PluginClassIndex *) pIndex.ptr;
+
+    if (index && index->initiated && pluginClassHandlerIndex == index->pcIndex)
     {
 	return static_cast<Tp *>
-	    (base->pluginClasses[mIndex.index]);
+	    (base->pluginClasses[index->index]);
     }
-    if (mIndex.failed && pluginClassHandlerIndex == mIndex.pcIndex)
+    if (!index || (index->failed && pluginClassHandlerIndex == index->pcIndex))
 	return NULL;
 
-    if (screen->hasValue (keyName ()))
+    if (index && screen->hasValue (keyName ()))
     {
-	mIndex.index     = screen->getValue (keyName ()).uval;
-	mIndex.initiated = true;
-	mIndex.failed    = false;
-	mIndex.pcIndex = pluginClassHandlerIndex;
-	return static_cast<Tp *> (base->pluginClasses[mIndex.index]);
+	index->index     = screen->getValue (keyName ()).uval;
+	index->initiated = true;
+	index->failed    = false;
+	index->pcIndex = pluginClassHandlerIndex;
+	return static_cast<Tp *> (base->pluginClasses[index->index]);
+    }
+    else if (index)
+    {
+	index->initiated = false;
+	index->failed    = true;
+	index->pcIndex = pluginClassHandlerIndex;
+	return NULL;
     }
     else
     {
-	mIndex.initiated = false;
-	mIndex.failed    = true;
-	mIndex.pcIndex = pluginClassHandlerIndex;
 	return NULL;
     }
 }
-- 
1.7.1

Places

File 0001-Use-dynamic-allocation-for-plugin-class-indexes-and-.patch of Package compiz (Revision 9a0c2cc6ccc7181ec639fbd1710d3f12)

Places