search

philippec / PhFacebook

MacOSX Interface to Facebook graph API
http://developer.casgrain.com/?p=107
Other
178 stars 44 forks source link

Constant crash. Memory management issue. #47

Closed Krivoblotsky closed 9 years ago

Krivoblotsky commented 9 years ago

I'm experiencing the constant EXC_BAD_ACCESS crash when using PhFacebook in PhAuthenticationToken.

shot

After I've investigated the source, I've found that you keep "expiry" as "assign" property, which means the retainCount will not be increased. Next you are setting [NSDate dateWithTimeIntervalSinceNow: seconds] which is already "autoreleased" object, so when dealloc is called you are trying to release _expiry again, that's cause the issue. 

....
@property (nonatomic, readonly) NSDate *expiry;
....
[_expiry release];
...

Can you, please, refactor the property to have "retain" attribute? Or even refactor the whole class to use property attributes, and get rid of [token copy] and [_permissions copy] in constructor.

...or just add "retain":

_expiry = [[NSDate dateWithTimeIntervalSinceNow: seconds] retain];

I can submit a pull request if you want.

philippec commented 9 years ago

That looks like a weird oversight on my part and is now fixed. Thanks!

As for refactoring the properties to be explicitly retain or copy, I don't think there is a need because they are declared as readonly. The naked ivar access done in -init and -dealloc is correct (now), and remains an implementation detail as far as the calling class goes.