pkyeck / socket.IO-objc

socket.io v0.7.2 — 0.9.x for iOS and OS X
MIT License
1.97k stars 439 forks source link

Fix to memory leak found during profiling application #199

Open jasonbodie opened 10 years ago

jasonbodie commented 10 years ago

During profiling we found a memory leak in our app. The Leak pointed to how the dispatch_source_t was being managed. These changes seemed to cleanly resolve the leak issue and are in line with Apple’s usage documentation.

jasonbodie commented 10 years ago

@sroze This is the updated pull request.

breaklee commented 10 years ago

this is available on ARC?

dispatch_release(_timeout); this statements is not allowed on ARC.

pkyeck commented 10 years ago

haven't tried it yet, but what I found: http://stackoverflow.com/questions/8618632/does-arc-support-dispatch-queues/8619055#8619055

seems like this PR would break everything above iOS 6.0 or Mac OS X 10.8

breaklee commented 10 years ago

thats good article. ill try it

maziyarpanahi commented 10 years ago

I talked about this in here: https://github.com/pkyeck/socket.IO-objc/pull/195

We have been seeing more memory leaks reports and I don't think there is any memory leaks in iOS7 by using ARC. I always check it and my usage is heavy.

If we sure there is no memory leaks in iOS7+ARC it should be something in reading about this in READ.ME to prevent reporting this issue over and over again.

jasonbodie commented 10 years ago

@maziyarpanahi I closed my original request prior to your earlier comments and created this request because I was asked to squash down to one commit. So essentially this is the original pull request not an "over and over again"...

This change does not cause problems when compiling and according to the apple documentation should, at worse, have no effect. Yet it does resolve the memory leak we are seeing in the profiler. I am also using iOS7 and ARC.