Unable to locate memory leak

Discussion in 'Mac Programming' started by tfoutz99, Apr 27, 2010.

  1. tfoutz99 macrumors newbie

    Joined:
    May 10, 2005
    #1
    I am trying to get unison to fire whenever a file changes in a certain directory. The whole of my code is:

    Code:
    #import <Foundation/Foundation.h>
    #include <CoreServices/CoreServices.h>
    
    
    void mycallback(ConstFSEventStreamRef streamRef,
    				void *clientCallBackInfo,
    				size_t numEvents,
    				void *eventPaths,
    				const FSEventStreamEventFlags eventFlags[],
    				const FSEventStreamEventId eventIds[]) {
    	sync();
    }
    
    void sync() {
    	//NSString* unisonPath = [[NSString alloc] initWithString: @"/usr/local/bin/unison"];
    
    	// ** Using /bin/ls for testing purposes **
    	NSString* unisonPath = [[NSString alloc] initWithString: @"/bin/ls"];
    	unisonPath = [unisonPath stringByStandardizingPath];
    	
    	NSTask *unison;
    	unison = [[NSTask alloc] init];
    	[unison setLaunchPath:(NSString *)unisonPath];
    	[unison launch];
    	[unison release];
    	[unisonPath release];
    }
    
    
    int main (int argc, const char * argv[]) {
    	// Based on http://cocoadevcentral.com/articles/000025.php
        NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
    	
    	NSString *syncPath = @"/Users/tommyboy/sync";
    	syncPath = [syncPath stringByStandardizingPath];
    	
    	/* Define variables and create a CFArray object containing
    	 CFString objects containing paths to watch.
         */
        CFStringRef mypath = (CFStringRef)syncPath;
        CFArrayRef pathsToWatch = CFArrayCreate(NULL, (const void **)&mypath, 1, NULL);
    	void *callbackInfo = NULL; // could put stream-specific data here.
        FSEventStreamRef stream;
        CFAbsoluteTime latency = 0; /* Latency in seconds */
    	
        /* Create the stream, passing in a callback */
        stream = FSEventStreamCreate(NULL,
    								 &mycallback,
    								 callbackInfo,
    								 pathsToWatch,
    								 kFSEventStreamEventIdSinceNow, /* Or a previous event ID */
    								 latency,
    								 kFSEventStreamCreateFlagNone /* Flags explained in reference */
    								 );
    	FSEventStreamScheduleWithRunLoop(stream, 
    									 CFRunLoopGetCurrent(),         
    									 kCFRunLoopDefaultMode);
    	FSEventStreamStart(stream);
    	CFRunLoopRun();
        [pool drain];
        return 0;
    }
    
    and then in terminal:
    for i in {1..10000}; do touch ~/sync/temp.txt; sleep 0.001; done

    Then in instruments, i use objectalloc on my process, and just watch the memory grow. Obviously, I would normally do a latency of ~30s, so this problem would occur much slower. However, the idea is to have this background app running constantly. I would love any suggestions! I have been coding with cocoa ~10 days, so I would also enjoy any feedback.
    --Tom
     
  2. jared_kipe macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #2
    Guaranteed to leak. unisonPath is alloc'd and init'd then stomped over the next assignment that returns an autoreleased string. The final [unisonPath release] is unnessicary, as it doesn't release the originally created string.

    How about...
    Code:
    void sync() {
    	//NSString* unisonPath = [[NSString alloc] initWithString: @"/usr/local/bin/unison"];
    
    	// ** Using /bin/ls for testing purposes **
    	NSString* unisonPath = [[NSString alloc] initWithString: @"/bin/ls"];
    	
    	NSTask *unison;
    	unison = [[NSTask alloc] init];
    	[unison setLaunchPath: [unisonPath stringByStandardizingPath]];
    	[unison launch];
    	[unison release];
    	[unisonPath release];
    }
    
    
     
  3. tfoutz99 thread starter macrumors newbie

    Joined:
    May 10, 2005
    #3
    Not sure if that helped

    Thanks for your reply!

    That may have fixed some of the problems, but I'm still getting a growing objectalloc.
    [​IMG]
     
  4. chown33 macrumors 604

    Joined:
    Aug 9, 2009
    #4
    My first thought was to add the code shown in red, and see if it changes anything.

    My second thought was to google NSPathStore2.

    My third thought was that the callback isn't waiting for the NSTask to terminate, so it seems possible that something associated with each NSTask isn't cleaned up until the task is waited for. Clues are NSPathStore2 count: 4581; Malloc 224 bytes count: 1527; CFMachPort count: 1527; 4581 / 1527 = 3; number of file-descriptors typically inherited by child processes: 3 (stdin, stdout, stderr).


    This seems needlessly redundant. What's wrong with:
    Code:
    NSString* unisonPath = @"/bin/ls";
    
    and remove the [unisonPath release] entirely.
     
  5. jared_kipe macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #5
    Two things, I'm concerned your autorelease pool doesn't ever drain the auto released objects. So first thing I'd do is rewrite it in a way that doesn't use any autoreleased objects OR create a new autorelease pool around them and drain the pool when you're done with them. Meaning make a new autorelease pool in sync() and drain it at the end of sync().

    Second, it doesn't seem like you are memory managing the CFArray you make, and possibly other CF type objects. However I'd doubt this would cause the "leak" to get worse.

    And I guess thirdly, can you run it with the tool "Leaks" to see if you can detect actual leaks and not?
     
  6. Mac Player macrumors regular

    Joined:
    Jan 19, 2006
    #6
    Erm, that instruments view shows the cumulative object allocation, of course tit is always growing!! Use Run with performance tool -> Leaks instead.
     
  7. tfoutz99 thread starter macrumors newbie

    Joined:
    May 10, 2005
    #7
    Getting closer

    It looks like both of you were correct. I made the following changes:
    Code:
    void mycallback(ConstFSEventStreamRef streamRef,
    				void *clientCallBackInfo,
    				size_t numEvents,
    				void *eventPaths,
    				const FSEventStreamEventFlags eventFlags[],
    				const FSEventStreamEventId eventIds[]) {
    	NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
    	sync();
        [pool drain];
    }
    
    void sync() {
    	NSTask *unison;
    	unison = [[NSTask alloc] init];
    	[unison setLaunchPath:@"/bin/ls"];
    	[unison launch];
    	[unison release];
    }
    
    The autoreleasepool helped, but objectalloc still seems to grow, though much slower now. And we got rid of the NSPathStore2. Now I just have that CFBasicHast. It doesn't grow on every run of sync(), but still grows.

    the Leaks test didn't find any leaks...
    [​IMG]

    Now I'm just showing living objects.

    Sidenote: why would there be a growing number of living objects, but no leaks?
     
  8. xUKHCx Administrator emeritus

    xUKHCx

    Joined:
    Jan 15, 2006
    Location:
    The Kop
    #8
    Mod note: Discussion moved to a new thread. It was previously located at the bottom of this thread
     
  9. tfoutz99 thread starter macrumors newbie

    Joined:
    May 10, 2005
    #9
    Simplified code snippet... Still has objectalloc problem

    I made a smaller program, easier to debug. I still get the same memory problem. I tried putting the entire function inside the for loop, and I then instead of objectalloc problems, I got leaks. :confused:

    Code:
    #import <Foundation/Foundation.h>
    
    void sync() {
    	NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
    	NSTask *unison;
    	unison = [[NSTask alloc] init];
    	[unison setLaunchPath:@"/bin/ls"];
    	[unison launch];
    	[unison release];
        [pool drain];
    }
    
    int main (int argc, const char * argv[]) {
        NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
    
    	int i;
        for (i=1; i<=5e4; i++) {
    		sync();
        }
        [pool drain];
        return 0;
    }
    
    [​IMG]
     
  10. jared_kipe macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #10
    May I suggest the following modifications.
    Doing so has reduced the growth of my Live Bytes significantly.

    Code:
    #import <Foundation/Foundation.h>
    
    void sync() {
    	NSTask *unison;
    	unison = [[NSTask alloc] init];
    	[unison setLaunchPath:@"/bin/ls"];
    	[unison launch];
    	[unison waitUntilExit];
    	[unison release];
    }
    
    int main (int argc, const char * argv[]) {
        NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
    
        int i;
        for (i=1; i<=5e4; i++) {
    		sync();
    		[pool drain];
        }
        [pool drain];
        return 0;
    }
    
    EDIT: Odd I tried making the autorelease pool inside the sync function after adding the waitUntilExit on the task and now my memory usage is basically constant.

    Code:
    #import <Foundation/Foundation.h>
    
    void sync() {
    	NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
    	NSTask *unison;
    	unison = [[NSTask alloc] init];
    	[unison setLaunchPath:@"/bin/ls"];
    	[unison launch];
    	[unison waitUntilExit];
    	[unison release];
    	[pool drain];
    }
    
    int main (int argc, const char * argv[]) {
        NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
    
        int i;
        for (i=1; i<=5e4; i++) {
    		sync();
        }
        [pool drain];
        return 0;
    }
    
    Possibly C-functions do not get to add objects to the applications main autorelease pool or something along those lines.
     
  11. tfoutz99 thread starter macrumors newbie

    Joined:
    May 10, 2005
    #11
    Thank you!

    That works beautifully. It stays at ~7 KB, rather than growing to who knows what.

    I just found the following comment on an old apple mailing list:
    Those comments suggested trying to solve the issue with an NSThread, which in my particular case would have probably been problematic. jared_kipe's solution, of creating an autoreleasepool in the subprocess and waitUntilExit before draining, is much more readily applicable in my program.

    Thanks for everyone's help!
    --Tom
     
  12. jared_kipe macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #12
    I found that too lol, however NSTask uses NSThread so who knows. What is odd is that you are not using any autoreleased objects in that specific code fragment, so presumably NSTask itself is using a whole lot of autoreleased objects and they are not getting into the original autoreleasepool because draining it on return from sync() doesn't help much.
     
  13. Mac Player macrumors regular

    Joined:
    Jan 19, 2006
    #13
    Okay here is the problem: your callback is ran from the main thread, and without app kit there is no extra pools created per event loop. This means that any autoreleased objects created inside sync are only being released with you drain the pool at the end of the program.

    jared_kipe, your first example doesn;t work because drain releases the pool! That means subsequent autoreleases from sync are being done with no pool in place!
     
  14. jared_kipe macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #14
    Ahh, good to know. So far I've only had use autoreleases and reference counting in my iPhone apps. I've never created and drained a pool myself.

    I'm AWARE that if you're creating and autoreleasing a lot of objects you should create another autorelease pool to help expedite the destruction of unused objects, but was not aware that draining a pool destroyed it. It seems like it would be useful to have a pool that doesn't get destroyed when drained, only sends release to all the objects in the pool.

    EDIT: Why doesn't the app crash then? If pool gets dealloc'd on my first drain, why do subsequent drains not crash the app?
     
  15. Mac Player macrumors regular

    Joined:
    Jan 19, 2006
  16. mif macrumors regular

    Joined:
    Feb 16, 2010
    Location:
    home
    #16
    I hope that there is always assembler and plain c. Good old days. Of course memory protection is very nice.

    Bugs which can bring whole system down, are not nice. Viruses are very dangerous too.

    Garbage collection is nice too, but it's very cool to handle memory itself.

    Just beginner.
     

Share This Page