1

In another question I asked as an aside, for confirmation that my use of NSStringconst char* in the code below is correct. I was asked to remove this as two questions were being asked in a single post.

The code that I have below works, but I can see that memory consumption grows monotonically, so I am concerned about a memory leak. Attempting to free the strings results in "pointer being freed was not allocated"; no experience in Objective C, but this segfault seems like the correct behaviour, so where is the leak?

#include <Cocoa/Cocoa.h>
#include <CoreGraphics/CGWindow.h>

struct details {
   int wid;
   int pid;
   const char* name;
   const char* window;
};

int activeWindow(struct details *d)
{
    if (d == NULL) {
        return 0;
    }
    NSArray *windows = (NSArray *)CGWindowListCopyWindowInfo(kCGWindowListExcludeDesktopElements|kCGWindowListOptionOnScreenOnly,kCGNullWindowID);
    for(NSDictionary *window in windows){
        int WindowLayer = [[window objectForKey:(NSString *)kCGWindowLayer] intValue];
        if (WindowLayer == 0) {
            d->wid = [[window objectForKey:(NSString *)kCGWindowNumber] intValue];
            d->pid = [[window objectForKey:(NSString *)kCGWindowOwnerPID] intValue];
            d->name = [[window objectForKey:(NSString *)kCGWindowOwnerName] UTF8String];
            d->window = [[window objectForKey:(NSString *)kCGWindowName] UTF8String];
            return 1;
        }
    }
    return 0;
}
kortschak
  • 755
  • 5
  • 21
  • 2
    Besides my answer below — I'm assuming this code is using automatic reference counting (ARC), but if for some reason not: `CGWindowListCopyWindowInfo` returns an allocated array which you would also need to `-release` – Itai Ferber Apr 30 '23 at 13:09

1 Answers1

1

Under the hood, NSStrings have three main possible representations:

  1. A "typical" NSString with UTF-16 backing
  2. An NSString with ASCII backing
  3. A tagged pointer string with encoded backing

The specifics of these representations isn't terribly important here, except that when you ask for the -UTF8String representation of one, it may need to perform some sort of conversion to get you UTF-8 data. In the case of (2), because ASCII is a strict subset of UTF-8, the string is allowed to hand you a pointer to its already-existing underlying storage; but in (1) and (3), the string must allocate space for UTF-8 data, because it doesn't yet exist.

Because the pointer returned to you from -UTF8String is a regular old C pointer, there isn't any lifetime information/management associated with it: the NSString can't know when you're "done" using it in order to free the memory. Because the string may also hand you a pointer to its direct underlying storage (and can't communicate to you via this API whether or not you are allowed to free the result or not), NSString has to hand you a pointer which you are never responsible for freeing.

So there's a bit of a conundrum here:

  • NSString may need to allocate memory to store UTF-8 data
  • NSString must hand you a pointer to that data, which you are not responsible for freeing
  • NSString can't know when you might be done using the pointer, so it can't free it prematurely
  • NSString shouldn't just leak the memory

The way this is handled is by allocating a block of memory which is then -autoreleased. You can read more about autorelease pools in the old "Advanced Memory Management Programming Guide" (or a briefer explanation in another answer of mine), but the gist:

  • An object which is autoreleased is placed in an "autorelease pool" for later
  • When the pool is "drained", all contained objects are released, and their memory is freed
  • Unless you create an autorelease pool yourself, or are in a context which has created a local autorelease pool, there is a global autorelease pool for your process
  • The global autorelease pool may only be freed at the end of process execution, so unless you put in extra work, you may see autoreleased objects get "leaked" until then

The solution, in your case, comes in two parts.

  1. From the "Discussion" section of the -UTF8String docs:

    This C string is a pointer to a structure inside the string object, which may have a lifetime shorter than the string object and will certainly not have a longer lifetime. Therefore, you should copy the C string if it needs to be stored outside of the memory context in which you use this property.

    If you're grabbing the -UTF8String and attempting to store it, you must create a copy of it which you can later free yourself; otherwise, if the underlying NSString is released, you'll be holding on to a dangling pointer

  2. If you want to ensure that autoreleased objects get cleaned up sooner, you can create a local autorelease pool which will clean them up; in your case, you can do this once per loop

For example:

// This will create an autorelease pool local to this scope.
// At the end of the scope, all autoreleased objects will be cleaned up.
@autoreleasepool {
    d->wid = [[window objectForKey:(NSString *)kCGWindowNumber] intValue];
    d->pid = [[window objectForKey:(NSString *)kCGWindowOwnerPID] intValue];

    const char *ownerName = [[window objectForKey:(NSString *)kCGWindowOwnerName] UTF8String];
    d->name = strdup(ownerName); // Or copy however you'd prefer

    const char *windowName = [[window objectForKey:(NSString *)kCGWindowName] UTF8String];
    d->window = strdup(windowName);
    return 1;
}

Later, you will need to make sure you free(d->name) and free(d->window) or else you'll really be leaking memory.

Itai Ferber
  • 28,308
  • 5
  • 77
  • 83
  • Thanks. That has lead me the right direction (nice answer). I found that I also need to `CFRelease(windows);` before return. – kortschak Apr 30 '23 at 21:29