Re: [AD] Patch for fixing missing NSAutoreleasePools

[Elias Pschernig <elias.pschernig@xxxxxxxxxx>]

On Fri, 25 May 2012 21:07:33 -0600
Trent Gamblin <trent@xxxxxxxxxx> wrote:

> I've applied it with some minor changes, haven't compiled it yet but
> I'm unsure of this:
> 
> diff -rupN old/src/macosx/system.m new/src/macosx/system.m
> --- old/src/macosx/system.m	2012-04-11 18:12:16.000000000 +0200
> +++ new/src/macosx/system.m	2012-05-23 19:00:33.000000000 +0200
> @@ -452,7 +452,7 @@ NSImage* NSImageFromAllegroBitmap(ALLEGR
>     al_unlock_bitmap(bmp);
>     [img addRepresentation:rep];
>     [rep release];
> -   return [img autorelease];
> +   return img;
>  }
>  
>  /* This works as long as there is only one screen */
> 

I just quickly compiled current SVN on my macbook and started the 3
demos from xcode, they all seem to work fine.

About the patch, basically doing this is wrong (for an object you do
not have an extra reference to):

[object autorelease]
[object release]

It's likely to cause a crash. There are three places calling
NSImageFromAllegroBitmap.

addons/image/macosx.m in _al_osx_save_image_f:

This calls [image release] which always should have crashed. Not sure
why it did not.

src/macosx/osxgl.m in _al_osx_create_mouse_cursor:

Same thing, it has [cursor_image release] so should have crashed.

src/macosx/osxgl.m in set_icon:

Does not release the returned image, so this should just have leaked.

I just committed a fix by removing the autorelease and adding an
explicit release in set_icon.

ex_icon as well as ex_mouse_cursor still work after that for me. Would
be good if someone who has an OSX which warns about the runtime leaks
could try as well though.