diff --git a/changes/2472.bugfix.rst b/changes/2472.bugfix.rst new file mode 100644 index 0000000000..8598a3505b --- /dev/null +++ b/changes/2472.bugfix.rst @@ -0,0 +1 @@ +Fix memory leaks for toga.Icon and toga.Image in the Cocoa backend. diff --git a/cocoa/src/toga_cocoa/icons.py b/cocoa/src/toga_cocoa/icons.py index 06ff3c0f07..7a942647d1 100644 --- a/cocoa/src/toga_cocoa/icons.py +++ b/cocoa/src/toga_cocoa/icons.py @@ -13,24 +13,24 @@ def __init__(self, interface, path): self.path = path try: # We *should* be able to do a direct NSImage.alloc.init...(), but if the - # image file is invalid, the init fails, and returns NULL - but we've - # created an ObjC instance, so when the object passes out of scope, Rubicon - # tries to free it, which segfaults. To avoid this, we retain result of the - # alloc() (overriding the default Rubicon behavior of alloc), then release - # that reference once we're done. If the image was created successfully, we - # temporarily have a reference count that is 1 higher than it needs to be; - # if it fails, we don't end up with a stray release. + # image file is invalid, the init fails, returns NULL, and releases the + # Objective-C object. Since we've created an ObjC instance, when the object + # passes out of scope, Rubicon tries to free it, which segfaults. + # To avoid this, we retain result of the alloc() (overriding the default + # Rubicon behavior of alloc), then release that reference once we're done. + # If the image was created successfully, we temporarily have a reference + # count that is 1 higher than it needs to be; if it fails, we don't end up + # with a stray release. image = NSImage.alloc().retain() self.native = image.initWithContentsOfFile(str(path)) if self.native is None: raise ValueError(f"Unable to load icon from {path}") finally: + # Calling `release` here disabled Rubicon's "release on delete" automation. + # We therefore add an explicit `release` call in __del__ if the NSImage was + # initialized successfully. image.release() - # Multiple icon interface instances can end up referencing the same native - # instance, so make sure we retain a reference count at the impl level. - self.native.retain() - def __del__(self): if self.native: self.native.release() diff --git a/cocoa/src/toga_cocoa/images.py b/cocoa/src/toga_cocoa/images.py index 901d426d40..687a5e8dff 100644 --- a/cocoa/src/toga_cocoa/images.py +++ b/cocoa/src/toga_cocoa/images.py @@ -23,31 +23,44 @@ class Image: def __init__(self, interface, path=None, data=None, raw=None): self.interface = interface + self._needs_release = False try: # We *should* be able to do a direct NSImage.alloc.init...(), but if the - # image file is invalid, the init fails, and returns NULL - but we've - # created an ObjC instance, so when the object passes out of scope, Rubicon - # tries to free it, which segfaults. To avoid this, we retain result of the - # alloc() (overriding the default Rubicon behavior of alloc), then release - # that reference once we're done. If the image was created successfully, we - # temporarily have a reference count that is 1 higher than it needs to be; - # if it fails, we don't end up with a stray release. + # image file is invalid, the init fails, returns NULL, and releases the + # Objective-C object. Since we've created an ObjC instance, when the object + # passes out of scope, Rubicon tries to free it, which segfaults. + # To avoid this, we retain result of the alloc() (overriding the default + # Rubicon behavior of alloc), then release that reference once we're done. + # If the image was created successfully, we temporarily have a reference + # count that is 1 higher than it needs to be; if it fails, we don't end up + # with a stray release. image = NSImage.alloc().retain() if path: self.native = image.initWithContentsOfFile(str(path)) if self.native is None: raise ValueError(f"Unable to load image from {path}") + else: + self._needs_release = True elif data: nsdata = NSData.dataWithBytes(data, length=len(data)) self.native = image.initWithData(nsdata) if self.native is None: raise ValueError("Unable to load image from data") + else: + self._needs_release = True else: self.native = raw finally: + # Calling `release` here disabled Rubicon's "release on delete" automation. + # We therefore add an explicit `release` call in __del__ if the NSImage was + # initialized successfully. image.release() + def __del__(self): + if self._needs_release: + self.native.release() + def get_width(self): return self.native.size.width