rs / SDSegmentedControl

A drop-in remplacement for UISegmentedControl that mimic iOS 6 AppStore tab controls
MIT License
1.2k stars 180 forks source link

Changed _items initialization to avoid EXC_BAD_ACCESS crash #13

Closed xzenon closed 11 years ago

xzenon commented 11 years ago

Somehow _items property is not propely initialized when directly accessed in commonInit. It then causes EXC_BAD_ACCESS crash when looping self._items in layoutSegments.

(I don't use Interface Builder and initialize control via [[SDSegmentedControl alloc] initWithItems:...])

xzenon commented 11 years ago

Also I think generally you should avoid to use underscored names for properties with auto-synthesizing as it makes ivars with double-undersored names which seems like not a good practice (http://stackoverflow.com/questions/9918567/single-and-double-underscore-difference-in-declaring-synthesize)

rs commented 11 years ago

It's a bad habit to access ivars thru their accessors in initializers/deallocator, as they would throw KVO events on a partially de/initialized object. About the double underscore, it's not really an issue with modern objc runtime, each class has its own namespace, so ivar conflicts are unlikely.

About your EXC_BAD_ACCESS crash, I don't really understand why initializing the _items variable thru its accessor would be safer than assigning it directly. Do you have an easy way to reproduce the problem?

xzenon commented 11 years ago

Yes, I know about accessing ivars in initializers/deallocs. I'm pretty sure that underscores should not be the problem here. I just renamed "_items" property to "items" to avoid double underscored names and checked again - issue is still there.

Actually I'm not sure too what is the reason of such behavior. Logically it shoud be ok. I created simple project to reproduce - you can grab it here http://db.tt/cPWYGWz6

rs commented 11 years ago

Ok I think I found the issue. NSMutableArray.array returns an autoreleased value (doesn't start with init, new or create) and should be replace by NSMutableArray.new to instruct ARC to retain it.

xzenon commented 11 years ago

Nice catch. Thank you!