adium-1.4 3084:a4bf2b677fea: Several fixes to handling of listOb...
commits at adium.im
commits at adium.im
Sat Oct 16 05:26:25 UTC 2010
details: http://hg.adium.im/adium-1.4/rev/a4bf2b677fea
revision: 3084:a4bf2b677fea
author: Evan Schoenberg
date: Sat Oct 16 00:10:23 2010 -0500
Several fixes to handling of listObject notification delays, which allow a group of updates to be handled as a block rather than individually to improve performance and appearance of the changes.
* `-[AIContactObserverManager endListObjectNotificationsDelay] now overrides a previous call to `-[[AIContactObserverManager delayListObjectNotificationsUntilInactivity]; if the last notification delay is ended, we immediately send out our notifications and trigger the updates
* Added `-[AIContactObserverManager endListObjectNotificationsDelaysImmediately]` which cancels out any nested delays and performs immediate updates - necessary if the changes made require an immediate reloading of contact list data to avoid the use of stale (NSOutlineView-caached) data.
* Documentation improvement
Refs #14294
Subject: adium-1.4 3085:e2f9861fd0bd: Dear evands from 2008,
details: http://hg.adium.im/adium-1.4/rev/e2f9861fd0bd
revision: 3085:e2f9861fd0bd
author: Evan Schoenberg
date: Sat Oct 16 00:12:41 2010 -0500
Dear evands from 2008,
The problem was that `-[AIContactObserverManager delayListObjectNotificationsUntilInactivity]` could lead to a lag before notifications went out to the rest of Adium about your changes. I've fixed this for you.
Love,
evands
P.S. You know that million dollar loan that Suntrust offers physicians to start their practice? Take it out, and buy AAPL. Trust me.
Subject: adium-1.4 3086:96486eecf57c: Added -[ESObjectWithProperties clearProxyObjects] so we can trigger the behavior noted in the name without having to await a deallocation
details: http://hg.adium.im/adium-1.4/rev/96486eecf57c
revision: 3086:96486eecf57c
author: Evan Schoenberg
date: Sat Oct 16 00:13:13 2010 -0500
Added -[ESObjectWithProperties clearProxyObjects] so we can trigger the behavior noted in the name without having to await a deallocation
Subject: adium-1.4 3087:dba0d3d40782: Fixed memory management issues with AIProxyListObject.
details: http://hg.adium.im/adium-1.4/rev/dba0d3d40782
revision: 3087:dba0d3d40782
author: Evan Schoenberg
date: Sat Oct 16 00:26:18 2010 -0500
Fixed memory management issues with AIProxyListObject.
* When `AIAbstractAccount` or `AIChat` calls `-[AIContactController accountDidStopTrackingContact:]`, immediately force notification of the change afterwards. (This is done by the calling object, rather than within accountDidStopTrackingContact itself, as multiple calls may be made in a row in the case of AIAccount). This in turn makes the contact list reload, which fixes the rare memory management crashes related to stale AIProxyListObjects.
* `AIProxyListObject` now retains its `listObject`; this does set up a retain loop, but we break the loop in `+[AIProxyListObject releaseProxyObject]` which is the sole route for deallocating an AIProxyListObject.
* Improved debug logging on the offchance the bug recurs
Fixes #14294. Fixes #14379. Fixes #14294. Fixes #13843. Fixes #13846. Refs #11896.
diffs (298 lines):
diff -r a960a9aa710d -r dba0d3d40782 Frameworks/Adium Framework/Source/AIAbstractAccount.m
--- a/Frameworks/Adium Framework/Source/AIAbstractAccount.m Thu Oct 14 23:13:50 2010 -0500
+++ b/Frameworks/Adium Framework/Source/AIAbstractAccount.m Sat Oct 16 00:26:18 2010 -0500
@@ -1273,17 +1273,12 @@
[self removePropertyValuesFromContact:listContact silently:YES];
}
- [[AIContactObserverManager sharedManager] endListObjectNotificationsDelay];
-
- [[AIContactObserverManager sharedManager] delayListObjectNotifications];
-
- //Stop track only afer notifying
for (AIListContact *listContact in myContacts) {
if (![adium.chatController existingChatWithContact:[listContact parentContact]])
[adium.contactController accountDidStopTrackingContact:listContact];
}
-
- [[AIContactObserverManager sharedManager] endListObjectNotificationsDelay];
+
+ [[AIContactObserverManager sharedManager] endListObjectNotificationsDelaysImmediately];
}
/*!
diff -r a960a9aa710d -r dba0d3d40782 Frameworks/Adium Framework/Source/AIChat.m
--- a/Frameworks/Adium Framework/Source/AIChat.m Thu Oct 14 23:13:50 2010 -0500
+++ b/Frameworks/Adium Framework/Source/AIChat.m Sat Oct 16 00:26:18 2010 -0500
@@ -671,7 +671,10 @@
if (contact.isStranger &&
![adium.chatController allGroupChatsContainingContact:contact.parentContact].count &&
![adium.chatController existingChatWithContact:contact.parentContact]) {
+
+ [[AIContactObserverManager sharedManager] delayListObjectNotifications];
[adium.contactController accountDidStopTrackingContact:contact];
+ [[AIContactObserverManager sharedManager] endListObjectNotificationsDelaysImmediately];
}
[inObject release];
diff -r a960a9aa710d -r dba0d3d40782 Frameworks/Adium Framework/Source/AIContactObserverManager.h
--- a/Frameworks/Adium Framework/Source/AIContactObserverManager.h Thu Oct 14 23:13:50 2010 -0500
+++ b/Frameworks/Adium Framework/Source/AIContactObserverManager.h Sat Oct 16 00:26:18 2010 -0500
@@ -46,6 +46,7 @@
- (void)updateContacts:(NSSet *)contacts forObserver:(id <AIListObjectObserver>)inObserver;
- (void)delayListObjectNotifications;
- (void)endListObjectNotificationsDelay;
+- (void)endListObjectNotificationsDelaysImmediately;
@property (readonly, nonatomic) BOOL updatesAreDelayed;
- (void)delayListObjectNotificationsUntilInactivity;
- (void)listObjectStatusChanged:(AIListObject *)inObject modifiedStatusKeys:(NSSet *)inModifiedKeys silent:(BOOL)silent;
diff -r a960a9aa710d -r dba0d3d40782 Frameworks/Adium Framework/Source/AIContactObserverManager.m
--- a/Frameworks/Adium Framework/Source/AIContactObserverManager.m Thu Oct 14 23:13:50 2010 -0500
+++ b/Frameworks/Adium Framework/Source/AIContactObserverManager.m Sat Oct 16 00:26:18 2010 -0500
@@ -35,6 +35,7 @@
@interface AIContactObserverManager ()
- (NSSet *)_informObserversOfObjectStatusChange:(AIListObject *)inObject withKeys:(NSSet *)modifiedKeys silent:(BOOL)silent;
- (void)_performDelayedUpdates:(NSTimer *)timer;
+ at property (nonatomic, retain) NSTimer *delayedUpdateTimer;
@end
#define UPDATE_CLUMP_INTERVAL 1.0
@@ -70,26 +71,68 @@
[contactObservers release]; contactObservers = nil;
[delayedModifiedStatusKeys release];
[delayedModifiedAttributeKeys release];
+ self.delayedUpdateTimer = nil;
[super dealloc];
}
//Status and Display updates -------------------------------------------------------------------------------------------
#pragma mark Status and Display updates
-//These delay Contact_ListChanged, ListObject_AttributesChanged, Contact_OrderChanged notificationsDelays,
-//sorting and redrawing to prevent redundancy when making a large number of changes
-//Explicit delay. Call endListObjectNotificationsDelay to end
+
+ at synthesize delayedUpdateTimer;
+
+/*!
+ * @brief Delay notifications for listObject changes until a matching endListObjectNotificationsDelay is called.
+ *
+ * This delays Contact_ListChanged, ListObject_AttributesChanged, Contact_OrderChanged notificationsDelays,
+ * sorting and redrawing to prevent redundancy when making a large number of changes.
+ *
+ * Each call must be paired with endListObjectNotificationsDelay. Nested calls are supported; notifications are sent
+ * when all delays have been ended.
+ */
- (void)delayListObjectNotifications
{
delayedUpdateRequests++;
+
updatesAreDelayed = YES;
}
-//End an explicit delay
+/*!
+ * @brief End a delay of notifications for listObject changes.
+ *
+ * This is paired with delayListObjectNotifications. Nested calls are supported; notifications are sent
+ * when all delays have been ended.
+ */
- (void)endListObjectNotificationsDelay
{
- delayedUpdateRequests--;
- if (delayedUpdateRequests == 0 && !delayedUpdateTimer) {
+ if (delayedUpdateRequests > 0) {
+ delayedUpdateRequests--;
+ if (delayedUpdateRequests == 0) {
+ if (delayedUpdateTimer)
+ [delayedUpdateTimer invalidate]; self.delayedUpdateTimer = nil;
+
+ [self _performDelayedUpdates:nil];
+ }
+ }
+}
+
+/*!
+ * @brief Immediately end all notifications for listObject changes.
+ *
+ * This ignores nested delayListObjectNotifications / endListObjectNotificationsDelay pairs and cancels
+ * all delays immediately. Subsequent calls to endListObjectNotificationsDelay (until delayListObjectNotifications is
+ * called) will be ignored.
+ *
+ * This is useful if changes are made that require an immediate update, regardless of what other code might want for
+ * efficiency. Notably, after deallocating AIListProxyObjects, the contact list *must* have reloadData called upon it
+ * (which occurs via its response to Contact_ListChanged sent via -[AIContactObserverManager _performDelayedUpdates:])
+ * or it may crash as it accesses deallocated objects as it does not retain the objects it displays.
+ */
+- (void)endListObjectNotificationsDelaysImmediately
+{
+ if (delayedUpdateRequests) {
+ delayedUpdateRequests = 0;
+ [self.delayedUpdateTimer invalidate]; self.delayedUpdateTimer = nil;
[self _performDelayedUpdates:nil];
}
}
@@ -102,11 +145,11 @@
{
if (!delayedUpdateTimer) {
updatesAreDelayed = YES;
- delayedUpdateTimer = [[NSTimer scheduledTimerWithTimeInterval:UPDATE_CLUMP_INTERVAL
- target:self
- selector:@selector(_performDelayedUpdates:)
- userInfo:nil
- repeats:YES] retain];
+ self.delayedUpdateTimer = [NSTimer scheduledTimerWithTimeInterval:UPDATE_CLUMP_INTERVAL
+ target:self
+ selector:@selector(_performDelayedUpdates:)
+ userInfo:nil
+ repeats:YES];
} else {
//Reset the timer
[delayedUpdateTimer setFireDate:[NSDate dateWithTimeIntervalSinceNow:UPDATE_CLUMP_INTERVAL]];
@@ -227,8 +270,7 @@
if (!delayedUpdateTimer || !updatesOccured) {
if (delayedUpdateTimer) {
[delayedUpdateTimer invalidate];
- [delayedUpdateTimer release];
- delayedUpdateTimer = nil;
+ self.delayedUpdateTimer = nil;
}
if (delayedUpdateRequests == 0) {
updatesAreDelayed = NO;
diff -r a960a9aa710d -r dba0d3d40782 Frameworks/Adium Framework/Source/AIProxyListObject.h
--- a/Frameworks/Adium Framework/Source/AIProxyListObject.h Thu Oct 14 23:13:50 2010 -0500
+++ b/Frameworks/Adium Framework/Source/AIProxyListObject.h Sat Oct 16 00:26:18 2010 -0500
@@ -22,12 +22,27 @@
@property (nonatomic, retain) NSString *cachedDisplayNameString;
@property (nonatomic, retain) NSAttributedString *cachedDisplayName;
@property (nonatomic) NSSize cachedDisplayNameSize;
- at property (nonatomic, assign) id listObject;
+
+/*!
+ * @brief Return the listObject represented by this AIProxyListObject
+ *
+ * This is a retain loop, as listObject also retains its AIProxyListObjects.
+ * It is therefore imperative that, when an AIListObject is no longer tracked by an account,
+ * +[AIProxyListObject releaseProxyObject:] is called. This must not wait until -[AIListContact dealloc] or it would
+ * never be called.
+ */
+ at property (nonatomic, retain) id listObject;
@property (nonatomic, assign) id <AIContainingObject> containingObject;
@property (nonatomic, retain) NSString *key;
+ (AIProxyListObject *)proxyListObjectForListObject:(ESObjectWithProperties *)inListObject
inListObject:(ESObjectWithProperties<AIContainingObject> *)containingObject;
+
+/*!
+ * @biref Called by ESObjectWithProperties to release its proxy object.
+ *
+ * This method resolves the retain count noted in documentation for -[AIPorxyListObject listObject]; it must be called.
+ */
+ (void)releaseProxyObject:(AIProxyListObject *)proxyObject;
@end
diff -r a960a9aa710d -r dba0d3d40782 Frameworks/Adium Framework/Source/AIProxyListObject.m
--- a/Frameworks/Adium Framework/Source/AIProxyListObject.m Thu Oct 14 23:13:50 2010 -0500
+++ b/Frameworks/Adium Framework/Source/AIProxyListObject.m Sat Oct 16 00:26:18 2010 -0500
@@ -40,7 +40,7 @@
// If the old list object is for some reason invalid (released in contact controller, but not fully released)
// we end up with an old list object as our proxied object. Correct this by getting rid of the old one.
#ifdef DEBUG_BUILD
- NSLog(@"Attempting to correct for old proxy listobject, keyed %@", key);
+ NSLog(@"Re-used AIProxyListObject (this should not happen.). Key %@ for inListObject %@ -> %p.listObject=%@", key,inListObject,proxy,proxy.listObject);
#endif
[proxy.listObject removeProxyObject:proxy];
[self releaseProxyObject:proxy];
@@ -70,11 +70,14 @@
+ (void)releaseProxyObject:(AIProxyListObject *)proxyObject
{
[[proxyObject retain] autorelease];
+ proxyObject.listObject = nil;
[proxyDict removeObjectForKey:proxyObject.key];
}
+
- (void)dealloc
{
+ self.listObject = nil;
self.key = nil;
self.cachedDisplayName = nil;
self.cachedDisplayNameString = nil;
@@ -111,4 +114,9 @@
return listObject;
}
+- (NSString *)description
+{
+ return [NSString stringWithFormat:@"<AIProxyListObject %p -> %@>", self, listObject];
+}
+
@end
diff -r a960a9aa710d -r dba0d3d40782 Frameworks/Adium Framework/Source/ESObjectWithProperties.h
--- a/Frameworks/Adium Framework/Source/ESObjectWithProperties.h Thu Oct 14 23:13:50 2010 -0500
+++ b/Frameworks/Adium Framework/Source/ESObjectWithProperties.h Sat Oct 16 00:26:18 2010 -0500
@@ -68,5 +68,6 @@
- (NSSet *)proxyObjects;
- (void)noteProxyObject:(id)proxyObject;
- (void)removeProxyObject:(id)proxyObject;
+- (void)clearProxyObjects;
@end
diff -r a960a9aa710d -r dba0d3d40782 Frameworks/Adium Framework/Source/ESObjectWithProperties.m
--- a/Frameworks/Adium Framework/Source/ESObjectWithProperties.m Thu Oct 14 23:13:50 2010 -0500
+++ b/Frameworks/Adium Framework/Source/ESObjectWithProperties.m Sat Oct 16 00:26:18 2010 -0500
@@ -45,18 +45,23 @@
return self;
}
+- (void)clearProxyObjects
+{
+ for (AIProxyListObject *proxy in proxyObjects)
+ [AIProxyListObject releaseProxyObject:proxy];
+ [proxyObjects release]; proxyObjects = nil;
+}
+
/*!
* @brief Deallocate
*/
- (void)dealloc
{
+ [self clearProxyObjects];
+
[propertiesDictionary release]; propertiesDictionary = nil;
[changedProperties release]; changedProperties = nil;
[displayDictionary release]; displayDictionary = nil;
-
- for (AIProxyListObject *proxy in proxyObjects)
- [AIProxyListObject releaseProxyObject:proxy];
- [proxyObjects release]; proxyObjects = nil;
[super dealloc];
}
diff -r a960a9aa710d -r dba0d3d40782 Source/AIContactController.m
--- a/Source/AIContactController.m Thu Oct 14 23:13:50 2010 -0500
+++ b/Source/AIContactController.m Sat Oct 16 00:26:18 2010 -0500
@@ -1245,19 +1245,12 @@
{
[[inContact retain] autorelease];
- /* Remove after a short delay. Otherwise, the removal may be visible as the object remains in the contact
- * list until a display delay is over, which would show up as the name going blank on metacontacts and other
- * odd behavior.
- *
- * Of course, this really means that the object delay code is somehow failing to actually delay all updates.
- * I can't figure out where or why, so this is a hack around it. Ugh. -evands 10/08
- */
for (AIListObject<AIContainingObject> *container in inContact.containingObjects) {
- [container performSelector:@selector(removeObjectAfterAccountStopsTracking:)
- withObject:inContact
- afterDelay:1];
+ [container removeObjectAfterAccountStopsTracking:inContact];
}
-
+
+ [inContact clearProxyObjects];
+
[contactDict removeObjectForKey:inContact.internalUniqueObjectID];
}
More information about the commits
mailing list