[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Weak references & GC
|
From: |
James Knight |
|
Subject: |
Weak references & GC |
|
Date: |
Wed, 19 Feb 2003 01:19:50 -0500 |
Hi, I recently have been hit with some crashing bugs in my app that
uses gnustep with GC, and traced it back to the fact that
NSNotificationCenter uses disguised pointers so that the GC doesn't
keep the referenced objects around. However, when the object
disappears, the observation is not cleared. In non-GC mode it's clearly
the user's responsibility to clear the observation in the -dealloc
method. However, in GC mode, I feel the system should be doing this for
you. The following patch accomplishes this, and does not affect non-GC
mode at all.
The main way it accomplishes this is via the
GC_general_register_disappearing_link function, which tells the GC to
clear a field when the referenced object is finalized. Most of the rest
of the code deals with purging the observations that are no longer
valid, and the changes at the end are to simply ignore the invalid
observations in _postAndRemove.
I've set it up to call _removeDeadObservations in addObserver, because
it's guaranteed that the table is writable and locked at that point. It
might be better to do the deletion in _postAndRemove, since it needs to
check for the invalid observers at that point anyways, but as I
understand it, you aren't guaranteed to be able to modify the table
during that function.
As far as I know, the only other place Cocoa/GNUstep uses weak
references is for delegates. I think a similar solution could be useful
for that case as well. I see there are also some references to
GS_GC_HIDE in GSTcpPort but I'm not sure what they're for. I'm not sure
I even understand why delegates are supposed to be weak references and
if that even actually needs to carry over into the world of
non-reference-counted GC. I can't think of any reason that I'd want a
delegate to disappear on me.
Comments?
James
Index: NSNotificationCenter.m
===================================================================
RCS file:
/cvsroot/gnustep/gnustep/core/base/Source/NSNotificationCenter.m,v
retrieving revision 1.37
diff -c -r1.37 NSNotificationCenter.m
*** NSNotificationCenter.m 3 Jan 2003 20:14:47 -0000 1.37
--- NSNotificationCenter.m 19 Feb 2003 04:39:14 -0000
***************
*** 433,438 ****
--- 433,443 ----
if (o->retained-- == 0)
{
NCTable *t = o->link;
+
+ #if GS_WITH_GC
+ GC_unregister_disappearing_link((GC_PTR*)&o->observer);
+ GC_unregister_disappearing_link((GC_PTR*)&o->selector);
+ #endif
o->link = (NCTable*)t->freeList;
t->freeList = o;
***************
*** 635,640 ****
--- 640,786 ----
*/
endNCTable(TABLE);
}
+
+ #if GS_WITH_GC
+ /*
+ * The following three functions deal with removing observations of
+ * or notifying dead objects by checking if selector or observer is
+ * nil, as setup by the GC_general_register_disappearing_link calls
+ * below.
+ */
+
+ /* This is the same as listPurge except that it removes
+ * all objects which have a nil observer or nil selector.
+ */
+ static Observation *listPurgeDead(Observation *list)
+ {
+ Observation *tmp;
+
+ while (list != ENDOBS && (list->observer == nil || list->selector
== 0))
+ {
+ tmp = list->next;
+ list->next = 0;
+ obsFree(list);
+ list = tmp;
+ }
+ if (list != ENDOBS)
+ {
+ tmp = list;
+ while (tmp->next != ENDOBS)
+ {
+ if (tmp->next->observer == nil || tmp->next->selector == 0)
+ {
+ Observation *next = tmp->next;
+
+ tmp->next = next->next;
+ next->next = 0;
+ obsFree(next);
+ }
+ else
+ {
+ tmp = tmp->next;
+ }
+ }
+ }
+ return list;
+ }
+
+ /*
+ * Utility function to remove all the observations from a particular
+ * map table node that are dead from their targets being GC'd.
+ * If the list of observations in the map node is emptied, the node is
+ * removed from the map.
+ */
+ static inline void
+ purgeDeadMapNode(GSIMapTable map, GSIMapNode node)
+ {
+ Observation *list = node->value.ext;
+
+ Observation *start = list;
+
+ list = listPurgeDead(list);
+ if (list == ENDOBS)
+ {
+ /*
+ * The list is empty so remove from map.
+ */
+ GSIMapRemoveKey(map, node->key);
+ }
+ else if (list != start)
+ {
+ /*
+ * The list is not empty, but we have changed its
+ * start, so we must place the new head in the map.
+ */
+ node->value.ext = list;
+ }
+ }
+
+ /*
+ * NOTE: this function assumes that the caller has already locked
+ * the table and ensured that it is not immutable!
+ */
+
+ - (void)_removeDeadObservations
+ {
+ /*
+ * NB. The removal algorithm depends on an implementation
characteristic
+ * of our map tables - while enumerating a table, it is safe to
remove
+ * the entry returned by the enumerator.
+ */
+
+ WILDCARD = listPurgeDead(WILDCARD);
+
+ GSIMapEnumerator_t e0;
+ GSIMapNode n0;
+
+ /*
+ * First try removing all named items set for this object.
+ */
+ e0 = GSIMapEnumeratorForMap(NAMED);
+ n0 = GSIMapEnumeratorNextNode(&e0);
+ while (n0 != 0)
+ {
+ GSIMapTable m = (GSIMapTable)n0->value.ptr;
+ NSString *thisName = (NSString*)n0->key.obj;
+
+ GSIMapEnumerator_t e1 = GSIMapEnumeratorForMap(m);
+ GSIMapNode n1 = GSIMapEnumeratorNextNode(&e1);
+
+ n0 = GSIMapEnumeratorNextNode(&e0);
+
+ while (n1 != 0)
+ {
+ GSIMapNode next = GSIMapEnumeratorNextNode(&e1);
+
+ purgeDeadMapNode(m, n1);
+ n1 = next;
+ }
+ /*
+ * If we removed all the observations keyed under this name, we
+ * must remove the map table too.
+ */
+ if (m->nodeCount == 0)
+ {
+ mapFree(TABLE, m);
+ GSIMapRemoveKey(NAMED, (GSIMapKey)thisName);
+ }
+ }
+
+ /*
+ * Now remove unnamed items
+ */
+ e0 = GSIMapEnumeratorForMap(NAMELESS);
+ n0 = GSIMapEnumeratorNextNode(&e0);
+ while (n0 != 0)
+ {
+ GSIMapNode next = GSIMapEnumeratorNextNode(&e0);
+
+ purgeDeadMapNode(NAMELESS, n0);
+ n0 = next;
+ }
+ }
+ #endif
/* Adding new observers. */
***************
*** 684,689 ****
--- 830,867 ----
o->observer = observer;
o->retained = 0;
o->next = 0;
+ #if GS_WITH_GC
+ /* Make sure that if the observer or object disappear, we clear
+ * the observation.
+ * HOWEVER: we can't use the field for 'object' directly, because
+ * a) it can already validly be nil, and
+ * b) doesn't exist except as a key for hashtable, and clearing
that
+ * would be a Bad Thing.
+ * Instead, clear the 'selector' field if 'object' is deallocated.
+ * Would be easier to just clear 'observer' field in both cases,
+ * but the GC API doesn't allow multiple objects per field.
+ *
+ * Thus, when observer or object are deallocated, observer or
+ * selector will become nil. This is checked and the observations
+ * dealloc'd in _removeDeadObservations, which we call before
+ * adding a new observation. This means invalid observations
+ * will persist until someone adds a new one, but I don't
+ * think it's safe to modify the observation list at
+ *_postAndRelease time, so this is the best way I could think
+ * to do it.
+ */
+
+ /* make sure the object is a real GC-allocated object with GC_base.
+ if it's not, and we try to watch it, the GC crashes. */
+ if(GC_base(observer) != 0)
+ GC_general_register_disappearing_link((GC_PTR*)&o->observer,
observer);
+ if(object != nil && GC_base(object) != 0)
+ GC_general_register_disappearing_link((GC_PTR*)&o->selector,
object);
+
+ /* Remove observations previously marked as dead from the GC */
+ [self _removeDeadObservations];
+
+ #endif
if (object != nil)
object = CHEATGC(object);
***************
*** 991,997 ****
*/
for (o = WILDCARD; o != ENDOBS; o = o->next)
{
! (*o->method)(o->observer, o->selector, notification);
}
/*
--- 1169,1178 ----
*/
for (o = WILDCARD; o != ENDOBS; o = o->next)
{
! #if GS_WITH_GC
! if (o->observer != nil && o->selector != 0)
! #endif
! (*o->method)(o->observer, o->selector, notification);
}
/*
***************
*** 1006,1012 ****
o = n->value.ext;
while (o != ENDOBS)
{
! (*o->method)(o->observer, o->selector, notification);
o = o->next;
}
}
--- 1187,1196 ----
o = n->value.ext;
while (o != ENDOBS)
{
! #if GS_WITH_GC
! if (o->observer != nil && o->selector != 0)
! #endif
! (*o->method)(o->observer, o->selector, notification);
o = o->next;
}
}
***************
*** 1039,1046 ****
o = n->value.ext;
while (o != ENDOBS)
{
! (*o->method)(o->observer, o->selector,
! notification);
o = o->next;
}
}
--- 1223,1233 ----
o = n->value.ext;
while (o != ENDOBS)
{
! #if GS_WITH_GC
! if (o->observer != nil && o->selector != 0)
! #endif
! (*o->method)(o->observer, o->selector,
! notification);
o = o->next;
}
}
***************
*** 1056,1063 ****
o = n->value.ext;
while (o != ENDOBS)
{
! (*o->method)(o->observer, o->selector,
! notification);
o = o->next;
}
}
--- 1243,1253 ----
o = n->value.ext;
while (o != ENDOBS)
{
! #if GS_WITH_GC
! if (o->observer != nil && o->selector != 0)
! #endif
! (*o->method)(o->observer, o->selector,
! notification);
o = o->next;
}
}
***************
*** 1079,1086 ****
while (count-- > arrayBase)
{
o = GSIArrayItemAtIndex(a, count).ext;
! if (o->next != 0)
! (*o->method)(o->observer, o->selector, notification);
}
GSIArrayRemoveItemsFromIndex(a, arrayBase);
--- 1269,1279 ----
while (count-- > arrayBase)
{
o = GSIArrayItemAtIndex(a, count).ext;
! #if GS_WITH_GC
! if (o->observer != nil && o->selector != 0)
! #endif
! if (o->next != 0)
! (*o->method)(o->observer, o->selector, notification);
}
GSIArrayRemoveItemsFromIndex(a, arrayBase);
***************
*** 1103,1110 ****
while (count-- > arrayBase)
{
o = GSIArrayItemAtIndex(a, count).ext;
! if (o->next != 0)
! (*o->method)(o->observer, o->selector, notification);
}
GSIArrayRemoveItemsFromIndex(a, arrayBase);
}
--- 1296,1306 ----
while (count-- > arrayBase)
{
o = GSIArrayItemAtIndex(a, count).ext;
! #if GS_WITH_GC
! if (o->observer != nil && o->selector != 0)
! #endif
! if (o->next != 0)
! (*o->method)(o->observer, o->selector, notification);
}
GSIArrayRemoveItemsFromIndex(a, arrayBase);
}
***************
*** 1163,1171 ****
while (count-- > arrayBase)
{
o = GSIArrayItemAtIndex(a, count).ext;
! if (o->next != 0)
! (*o->method)(o->observer, o->selector,
! notification);
}
GSIArrayRemoveItemsFromIndex(a, arrayBase);
}
--- 1359,1370 ----
while (count-- > arrayBase)
{
o = GSIArrayItemAtIndex(a, count).ext;
! #if GS_WITH_GC
! if (o->observer != nil && o->selector != 0)
! #endif
! if (o->next != 0)
! (*o->method)(o->observer, o->selector,
! notification);
}
GSIArrayRemoveItemsFromIndex(a, arrayBase);
}
- Weak references & GC,
James Knight <=