[commit] r2687 - in trunk: GME/Mga Tests/GPyUnit

GMESRC Repository Notifications gme-commit at list.isis.vanderbilt.edu
Wed Jul 19 15:50:33 CDT 2017


Author: ksmyth
Date: Wed Jul 19 15:50:33 2017
New Revision: 2687

Log:
Fix CMgaAttribute => CMgaFCO => apool reference cycle

Reverts most of "Fix write-after-free when IMgaFCO had final release before its IMgaAttribute"

Modified:
   trunk/GME/Mga/MgaAttribute.cpp
   trunk/GME/Mga/MgaAttribute.h
   trunk/Tests/GPyUnit/test_MetaInterpreter.py

Modified: trunk/GME/Mga/MgaAttribute.cpp
==============================================================================
--- trunk/GME/Mga/MgaAttribute.cpp	Tue Jul 18 14:10:44 2017	(r2686)
+++ trunk/GME/Mga/MgaAttribute.cpp	Wed Jul 19 15:50:33 2017	(r2687)
@@ -989,8 +989,12 @@
 }
 	
 
-CMgaPart::CMgaPart()	: next(NULL), load_status(ATTSTATUS_INVALID) {	}
-CMgaPart::~CMgaPart() {
+CMgaPart::CMgaPart()	: prevptr(NULL), next(NULL), load_status(ATTSTATUS_INVALID) {	}
+CMgaPart::~CMgaPart() {						// remove object from hash
+		if (next)
+			next->prevptr = prevptr;
+		if (prevptr)
+			*prevptr = next;
 }
 void CMgaPart::Initialize(metaref_type mr, ::FCO *o, CMgaProject *p) {   // Throws!!!
 		mref = mr;		

Modified: trunk/GME/Mga/MgaAttribute.h
==============================================================================
--- trunk/GME/Mga/MgaAttribute.h	Tue Jul 18 14:10:44 2017	(r2686)
+++ trunk/GME/Mga/MgaAttribute.h	Wed Jul 19 15:50:33 2017	(r2687)
@@ -27,7 +27,7 @@
 	public ISupportErrorInfoImpl<&__uuidof(IMgaAttribute)>
 {
 public:
-	CMgaAttribute()	: next(NULL), load_status(ATTSTATUS_INVALID) {	}
+	CMgaAttribute()	: prevptr(NULL), next(NULL), load_status(ATTSTATUS_INVALID) {	}
 
 DECLARE_PROTECT_FINAL_CONSTRUCT()
 
@@ -76,8 +76,13 @@
 
 	STDMETHOD(Clear)();
 
-	CComPtr<CMgaAttribute> next;
-	~CMgaAttribute() {
+	typedef CMgaAttribute *hashobp;
+	hashobp *prevptr, next;
+	~CMgaAttribute() {						// remove object from hash
+		if (next)
+			next->prevptr = prevptr;
+		if (prevptr)
+			*prevptr = next;
 	}
 	void Initialize(metaref_type mr, FCO *o, CMgaProject *p);   // Throws!!!
 	metaref_type mref;
@@ -106,30 +111,32 @@
 
 
 class attrpool {
-	CComPtr<CMgaAttribute> pool[APOOL_HASHSIZE];
+	// does not AddRef. CMgaAttribute.CoreObj holds a reference to (COM aggregate) CMgaFCO, which contains attrpool
+	CMgaAttribute::hashobp pool[APOOL_HASHSIZE];
 public:
 	attrpool() { 
+		int i; 
+		for(i = 0; i < APOOL_HASHSIZE;i++) pool[i] = NULL;
 	}
 
 	~attrpool() {
-		// clear should be called before destructor
 		int i; 
-		for (i = 0; i < APOOL_HASHSIZE; i++)
-			ASSERT(pool[i] == NULL);
+		for(i = 0; i < APOOL_HASHSIZE;i++) ASSERT(pool[i] == NULL);
 	}
 
 	// Throws (allocates)!!!!
 	CComPtr<IMgaAttribute> getpoolobj(metaref_type mref, FCO *o, CMgaProject *pr) {
-		CComPtr<CMgaAttribute> &k = pool[apool_hash(mref)];
-		CMgaAttribute **kk;
-		for (kk = &k; *kk != NULL; kk = &((*kk)->next)) {
+		CMgaAttribute::hashobp &k = pool[apool_hash(mref)], *kk;
+		for(kk = &k; *kk != NULL; kk = &((*kk)->next)) {
 			if((*kk)->mref == mref) {
 				return (*kk);
 			}
 		}
 		CComPtr<CMgaAttribute> s;
 		CreateComObject(s);
+		s->prevptr = &k;				// Insert to the front
 		s->next = k;
+		if(k) k->prevptr = &(s->next);
 		k = s;
 
 		s->Initialize(mref, o, pr);  
@@ -141,13 +148,13 @@
 	{
 		for (size_t mref = 0; mref < sizeof(pool) / sizeof(pool[0]); mref++)
 		{
-			CComPtr<CMgaAttribute> next;
-			for (next = pool[mref]; next != NULL; )
+			CMgaAttribute::hashobp current, next;
+			for (current = pool[mref]; current != NULL; current = next)
 			{
-				CComPtr<CMgaAttribute> removed = next;
-				next = removed->next;
-				removed->load_status = ATTSTATUS_INVALID;
-				removed->next = NULL;
+				next = current->next;
+				current->load_status = ATTSTATUS_INVALID;
+				current->prevptr = NULL;
+				current->next = NULL;
 			}
 			pool[mref] = NULL;
 		}
@@ -287,7 +294,8 @@
 	CMgaPart();
 	~CMgaPart();
 	void Initialize(metaref_type mr, ::FCO *o, CMgaProject *p);
-	CComPtr<CMgaPart> next;
+	typedef CMgaPart *hashobp;
+	hashobp *prevptr, next;
 	metaref_type mref;
 
 	long load_status;
@@ -313,41 +321,31 @@
 
 
 class partpool {
-	CComPtr<CMgaPart> pool[PPOOL_HASHSIZE];
+	CMgaPart::hashobp pool[PPOOL_HASHSIZE];
 public:
 	partpool() { 
+		int i; 
+		for(i = 0; i < PPOOL_HASHSIZE;i++) pool[i] = NULL;
 	}
 
 	~partpool() {
-		for (size_t mref = 0; mref < sizeof(pool) / sizeof(pool[0]); mref++)
-		{
-			CComPtr<CMgaPart> next;
-			for (next = pool[mref]; next != NULL; )
-			{
-				CComPtr<CMgaPart> removed = next;
-				next = removed->next;
-				removed->load_status = ATTSTATUS_INVALID;
-				removed->next = NULL;
-			}
-			pool[mref] = NULL;
-		}
 		int i; 
-		for (i = 0; i < PPOOL_HASHSIZE; i++)
-			ASSERT(pool[i] == NULL);
+		for(i = 0; i < PPOOL_HASHSIZE;i++) ASSERT(pool[i] == NULL);
 	}
 
 	// Throws (allocates)!!!!
 	CComPtr<IMgaPart> getpoolobj(metaref_type mref, FCO *o, CMgaProject *pr) {
-		CComPtr<CMgaPart> &k = pool[ppool_hash(mref)];
-		CMgaPart **kk;
-		for (kk = &k; *kk != NULL; kk = &((*kk)->next)) {
+		CMgaPart::hashobp &k = pool[ppool_hash(mref)], *kk;
+		for(kk = &k; *kk != NULL; kk = &((*kk)->next)) {
 			if((*kk)->mref == mref) {
 				return (*kk);
 			}
 		}
 		CComPtr<CMgaPart > s;
 		CreateComObject(s);
+		s->prevptr = &k;				// Insert to the front
 		s->next = k;
+		if(k) k->prevptr = &(s->next);
 		k = s;
 
 		s->Initialize(mref, o, pr);  

Modified: trunk/Tests/GPyUnit/test_MetaInterpreter.py
==============================================================================
--- trunk/Tests/GPyUnit/test_MetaInterpreter.py	Tue Jul 18 14:10:44 2017	(r2686)
+++ trunk/Tests/GPyUnit/test_MetaInterpreter.py	Wed Jul 19 15:50:33 2017	(r2687)
@@ -31,6 +31,28 @@
             self.assertEqual(atomattrs, list(atomattributes.split()))
         finally:
             metaproj.Close()
+
+    def test_attrpool_clear(self):
+        mga = GPyUnit.util.parse_xme(self.connstr)
+        # mga = DispatchEx("Mga.Mgaproject")
+        # mga.Open(self.connstr)
+        try:
+            mga.Save()
+
+            mga.BeginTransactionInNewTerr()
+            try:
+                obj = mga.RootFolder.GetObjectByPathDisp("/@Stereotypes/@AtomProxy")
+                attrs = list(obj.Attributes)
+                # print [a.Meta.MetaRef for a in attrs]
+                # add obj to CMgaProject.changedobjs
+                obj.SetStrAttrByNameDisp("Decorator", "asdf")
+            finally:
+                # abort calls changedobjs.front()->apool.clear()
+                # Before 6/26/2017: Under appverif with Heaps (full): "Win32 exception occurred releasing IUnknown at 0x503dcfc8"
+                mga.AbortTransaction()
+            del(attrs)
+        finally:
+            mga.Close()
     
     def _rm_old_files(self):
         for file in ("MetaGME.xmp", "MetaGME.mta", "MetaGME.xmp.log"):


More information about the gme-commit mailing list