Le 8 janv. 2014 à 14:02, Richard Frith-Macdonald a écrit : > On 8 Jan 2014, at 12:34, Mathias Bauer <[email protected]> wrote: > >> Am 08.01.14 13:21, schrieb Quentin Mathé: >>> >>> Le 8 janv. 2014 à 10:45, Mathias Bauer a écrit : >>> >>>> it seems that the implementation of countByEnumeratingWithState in NSArray >>>> is broken. >>>> >>>> The following code in NSArray.m >>>> >>>>> { >>>>> NSUInteger size = [self count]; >>>>> NSInteger count; >>>>> >>>>> /* This is cached in the caller at the start and compared at each >>>>> * iteration. If it changes during the iteration then >>>>> * objc_enumerationMutation() will be called, throwing an exception. >>>>> */ >>>>> state->mutationsPtr = (unsigned long *)size; >>>> >>>> of course crashes as soon as any fast enumeration is executed for any >>>> collection deriving from NSArray. The cast in the last line can't work. >>>> >>>> Now I'm wondering how this problem could remain undiscovered or at least >>>> unfixed for such a long time. I doubt that everybody who implemented a >>>> class that derives from NSArray also re-implemented this method. >>> >>> I just stumbled on it today while testing some custom NSArray subclass. I >>> think most people don't write NSArray subclass, and GNUstep concrete >>> subclasses are all overriding the fast enumeration method, so the default >>> fast enumeration implementation in NSArray was just never executed. >>> >>>> A simple fix would be to add an iVar that gets the result of [self count] >>>> each time this method is called and assigning its address to >>>> state->mutationsPtr. >>> >>> The following should be enough to fix it: state->mutationsPtr = (unsigned >>> long *)&size; > > I don't use fast enumeration, so I'm unfamiliar with this, but on the basis > of looking at this code and reading a little it also seems strange to me to > use a stack variable, unless you can somehow guarantee the stack will be left > intact while the fast enumeration takes place?
Yes, using a stack variable doesn't make sense. state->mutationsPtr would probably point to some random content in memory once -countWithEnumeratingWithState: returns. > I wonder if the pointer should simply be set to self? That would presumably > point to something guaranteed to exist for the duration fo the enumeration > and presumably to remain unchanged. This ought to be fine for NSArray which > is never mutated. That sounds reasonable. > I don't understand the point of using an ivar though. Leaving aside the fact > that NSArray is an abstract class and is not supposed/allowed to have ivars, > what would be the point? As I understand it the pointer is supposed to be set > to point to some value which will change if the collection is mutated ... > that should never happen for NSArray and if someone implements an > NSMutableArray subclass they would presumably want to implement their own > storage and mechanism to indicate when their contents had changed. So it > looks to me like subclasses of NSMutableArray ought to be implementing their > own version of this method. > > Of course, you may know better ... have Apple added an API to NSMutableArray > which allows a subclass to mark instances of itsself as mutated? It seems there is no such API. I was suggesting in my previous reply to move the _version ivar from GSMutableArray to NSMutableArray, but as you pointed it out, this won't help since overriden primitive methods (e.g. -addObject:) need to increment this variable. And Apple doesn't declare any similar protected ivar or special method for this purpose. I tested writing a custom NSMutableArray subclass and mutating it during fast enumeration on Mac OS X, and I get no mutation-related exceptions, so I guess NSMutableArray inherit -countByEnumeratingWithState: from NSArray in Cocoa (if you don't override it). Cheers, Quentin. _______________________________________________ Discuss-gnustep mailing list [email protected] https://lists.gnu.org/mailman/listinfo/discuss-gnustep
