diff --git a/ChangeLog b/ChangeLog index 2232d8bb7..b7a4c0897 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,13 @@ +2013-11-01 Richard Frith-Macdonald + + * Source/GSAttributedString.m: Use exact equality test when removing + attribute dictionary from cache. Prevents possibly removing the + wrong dictionary in cases where dictionary contents are mutated + while in the cache, such that two dictionaries become equal. + 2013-10-30 Richard Frith-Macdonald - * Source/GSDictionary.m: Add priovate class GSCachedDictionary which + * Source/GSDictionary.m: Add private class GSCachedDictionary which will raise an exception if deallocated without being uncached. * Source/GSAttributedString.m: Use GSCachedDictionary for attributes to aid in detection of things improperly releasing them. diff --git a/Source/GSAttributedString.m b/Source/GSAttributedString.m index d5e13f525..c35f8a4e5 100644 --- a/Source/GSAttributedString.m +++ b/Source/GSAttributedString.m @@ -95,11 +95,31 @@ +static BOOL adding; + +/* When caching attributes we make a shallow copy of the dictionary cached, + * so that it is immutable and safe to cache. + * However, we have a potential problem if the objects within the attributes + * dictionary are themselves mutable, and something mutates them while they + * are in the cache. In this case we could items added while different and + * then mutated to have the same contents, so we would not know which of + * the equal dictionaries to remove. + * The solution is to require dictionaries to be identical for removal. + */ +static inline BOOL +cacheEqual(id A, id B) +{ + if (YES == adding) + return [A isEqualToDictionary: B]; + else + return A == B; +} + #define GSI_MAP_RETAIN_KEY(M, X) #define GSI_MAP_RELEASE_KEY(M, X) #define GSI_MAP_RETAIN_VAL(M, X) #define GSI_MAP_RELEASE_VAL(M, X) -#define GSI_MAP_EQUAL(M, X,Y) [(X).obj isEqualToDictionary: (Y).obj] +#define GSI_MAP_EQUAL(M, X,Y) cacheEqual((X).obj, (Y).obj) #define GSI_MAP_KTYPES GSUNION_OBJ #define GSI_MAP_VTYPES GSUNION_NSINT #define GSI_MAP_NOCLEAN 1 @@ -140,6 +160,7 @@ cacheAttributes(NSDictionary *attrs) GSIMapNode node; ALOCK(); + adding = YES; node = GSIMapNodeForKey(&attrMap, (GSIMapKey)((id)attrs)); if (node == 0) { @@ -173,6 +194,7 @@ unCacheAttributes(NSDictionary *attrs) id removed = nil; ALOCK(); + adding = NO; bucket = GSIMapBucketForKey(&attrMap, (GSIMapKey)((id)attrs)); if (bucket != 0) { @@ -854,6 +876,7 @@ SANITY(); while (next < arraySize) { GSAttrInfo *n = OBJECTAT(next); + if (n->loc <= NSMaxRange(range)) { REMOVEAT(arrayIndex); @@ -892,7 +915,7 @@ SANITY(); && effectiveRange.length == range.length) { arrayIndex--; - if (arrayIndex!=0 || arraySize > 1) + if (arrayIndex != 0 || arraySize > 1) { REMOVEAT(arrayIndex); arraySize--;