ravish21 / oauthconsumer

Automatically exported from code.google.com/p/oauthconsumer
0 stars 0 forks source link

Mixed-mode memory management patch #4

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
OAuthConsumer appears to have been written to assume GC, and leaks exuberantly 
when run in a 
non-GC environment. I've got a patch to add mixed-mode memory management, which 
was 
pleasantly straightforward to put together. I really appreciate how clearly 
written the framework is, 
and I've tried not to gum that up.

I opted to use properties for accessor generation, and in a couple of places 
needed read-write in 
places where you'd specified read-only. I compromised by making those members 
private if they 
weren't marked as such already. Here's a summary of the property-related 
changes:

OAMutableURLRequest: signature and nonce moved from protected to private, their 
properties 
changed from read-only to read-write. Properties added for consumer, token, 
realm, signatureProvider, and timestamp. Added "self." to ivar usage in .m to 
go through accessors.
OADataFetcher - Properties added for request, responseData, and delegate. The 
'delegate' property 
retains its ivar, which is a little questionable, but I went with it to match 
the GC behavior. "self." usage 
added.
OAServiceTicket: ivars were already private, property access changed to 
read-write. 'self.' usage 
added.
OAConsumer, OAToken, OARequestParameter: Properties were in place, no changes 
needed.
OAHMAC_SHA1SignatureProvider, OAPlaintextSignatureProvider: No ivars, no 
changes needed.

Here's a summary of the other implementation changes in 
r678-memory-management.diff:

OAConsumer.m, OADataFetcher.m, OAHMAC_SHA1SignatureProvider.m, 
OAPlaintextSignatureProvider.m, OARequestParameter.m, OAServiceTicket.m: Added 
dealloc only.

OAMutableURLRequest.m - Added dealloc, fixed two CF leaks in _generateNonce, 
additional 
autoreleasing in _signatureBaseString.
OAToken.m - Added dealloc, reformatted initWithUserDefaults[...], 
storeInUserDefaults[...].
OAToken_KeychainExtensions.m - Changed NSMakeCollectable to CFRelease (outside 
GC, 
NSMakeCollectable's returned id objects aren't marked autoreleased).
NSMutableURLRequest+Parameters.m - Autoreleasing a couple of things.

I also ran into the same URL encoding problem that Mike fixed, and made some 
further changes 
there, which are in r678-url-encoding.diff. In NSString+URLEncoding.m, 

* CFStringRefs are now returned as NSStrings using NSMakeCollectable, which 
should plug leaks 
under GC. Returned strings are autoreleased, which plugs leaks not under GC. 

* I applied the same character overrides to encodedURLString as 
encodedURLParameterString, 
because I couldn't figure out why they should be different, given that section 
9.1.3 of oauth's spec 
references the parameter encoding section. (If the overrides are properly the 
same, perhaps the two 
encoding methods should be merged.) 

* I trimmed three chars out of Mike's character overrides, which testing 
indicates the CF encoding 
method knows about by default.

In NSString+URLEncodingTest.m, Mike's overridden characters are appended to the 
encoding test. 

Tested under Intel, all tests passed, no warnings. My OAuth app is just an 
infant but it exercises most 
of OAuthConsumer's functionality, and Instruments says I'm now leak-free across 
my code and the 
framework. I realize it's a large patch, and if I can do anything to answer 
questions or make it easier 
to digest, please let me know.

Thanks very much, it was a huge help not to have to learn the protocol from 
scratch.

Original issue reported on code.google.com by bumppo@gmail.com on 20 Oct 2008 at 9:55

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by jon.r.cr...@gmail.com on 23 Oct 2008 at 6:41