Le 8 janv. 2014 à 14:02, Richard Frith-Macdonald a écrit :
On 8 Jan 2014, at 12:34, Mathias Bauer <mathias_bauer@gmx.net> 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).