Navigation Menu

Skip to content
This repository has been archived by the owner on Feb 11, 2021. It is now read-only.

Commit

Permalink
Fix for double free when quitting on iOS
Browse files Browse the repository at this point in the history 
Tim Angus 2012-02-20 09:40:35 PST

As alluded to in the email thread "SDL2 error on iOS (doublefree)", I believe
the original cause of this bug is confusion over the purpose of
SDL_VideoDisplay::current_mode. It looks as though it is a weak reference to
another mode, albeit with value semantics. The iOS port treated it as a strong
reference however and claimed ownership, which is why things blew up. All the
patch really does it to stop treating current_mode as a strong reference.

To prevent this happening again it might be an idea to change current_mode to
be a pointer type rather than a value. This would certainly make its semantics
much more obvious. Failing that, a comment in the struct indicating its weak
reference properties might be wise.
  • Loading branch information
@slouken
slouken committed Feb 21, 2012
1 parent 22733ef commit 5d2fff5
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 51 deletions.
100 changes: 51 additions & 49 deletions src/video/uikit/SDL_uikitvideo.m
View file
Expand Up @@ -121,14 +121,11 @@ static void UIKit_DeleteDevice(SDL_VideoDevice * device)
*/

static int
UIKit_AddSingleDisplayMode(SDL_VideoDisplay * display, int w, int h,
UIKit_AllocateDisplayModeData(SDL_DisplayMode * mode,
UIScreenMode * uiscreenmode, CGFloat scale)
{
SDL_DisplayMode mode;
SDL_zero(mode);

SDL_DisplayModeData *data = NULL;

if (uiscreenmode != nil) {
/* Allocate the display mode data */
data = (SDL_DisplayModeData *) SDL_malloc(sizeof(*data));
Expand All @@ -138,24 +135,56 @@ static void UIKit_DeleteDevice(SDL_VideoDevice * device)
}

data->uiscreenmode = uiscreenmode;
[data->uiscreenmode retain];

data->scale = scale;
}

mode->driverdata = data;

return 0;
}

static void
UIKit_FreeDisplayModeData(SDL_DisplayMode * mode)
{
if (!SDL_UIKit_supports_multiple_displays) {
// Not on at least iPhoneOS 3.2 (versions prior to iPad).
SDL_assert(mode->driverdata == NULL);
} else if (mode->driverdata != NULL) {
SDL_DisplayModeData *data = (SDL_DisplayModeData *)mode->driverdata;
[data->uiscreenmode release];
SDL_free(data);
mode->driverdata = NULL;
}
}

static int
UIKit_AddSingleDisplayMode(SDL_VideoDisplay * display, int w, int h,
UIScreenMode * uiscreenmode, CGFloat scale)
{
SDL_DisplayMode mode;
SDL_zero(mode);

mode.format = SDL_PIXELFORMAT_ABGR8888;
mode.refresh_rate = 0;
mode.driverdata = data;
if (UIKit_AllocateDisplayModeData(&mode, uiscreenmode, scale) < 0) {
return -1;
}

mode.w = w;
mode.h = h;
if (SDL_AddDisplayMode(display, &mode)) {
if (uiscreenmode != nil) {
[uiscreenmode retain];
}

return 0;
}

SDL_free(data);
// Failure case; free resources
SDL_DisplayModeData *data = (SDL_DisplayModeData *) mode.driverdata;

if (data != NULL) {
[data->uiscreenmode release];
SDL_free(data);
}

return -1;
}
Expand Down Expand Up @@ -237,39 +266,27 @@ static void UIKit_DeleteDevice(SDL_VideoDevice * device)
mode.w = (int)(size.width * scale);
mode.h = (int)(size.height * scale);
mode.refresh_rate = 0;


UIScreenMode * uiscreenmode = nil;
// UIScreenMode showed up in 3.2 (the iPad and later). We're
// misusing this supports_multiple_displays flag here for that.
if (SDL_UIKit_supports_multiple_displays) {
SDL_DisplayModeData *data;

/* Allocate the mode data */
data = (SDL_DisplayModeData *) SDL_malloc(sizeof(*data));
if (!data) {
SDL_OutOfMemory();
return -1;
}

data->uiscreenmode = [uiscreen currentMode];
[data->uiscreenmode retain]; // once for the desktop_mode
[data->uiscreenmode retain]; // once for the current_mode

data->scale = scale;

mode.driverdata = data;
uiscreenmode = [uiscreen currentMode];
}

if (UIKit_AllocateDisplayModeData(&mode, uiscreenmode, scale) < 0) {
return -1;
}

SDL_zero(display);
display.desktop_mode = mode;
display.current_mode = mode;

SDL_DisplayData *data;

/* Allocate the display data */
data = (SDL_DisplayData *) SDL_malloc(sizeof(*data));
SDL_DisplayData *data = (SDL_DisplayData *) SDL_malloc(sizeof(*data));
if (!data) {
SDL_free(mode.driverdata);
SDL_OutOfMemory();
UIKit_FreeDisplayModeData(&display.desktop_mode);
return -1;
}

Expand Down Expand Up @@ -341,20 +358,6 @@ static void UIKit_DeleteDevice(SDL_VideoDevice * device)
return 0;
}

static void
UIKit_ReleaseUIScreenMode(SDL_DisplayMode * mode)
{
if (!SDL_UIKit_supports_multiple_displays) {
// Not on at least iPhoneOS 3.2 (versions prior to iPad).
SDL_assert(mode->driverdata == NULL);
} else {
SDL_DisplayModeData *data = (SDL_DisplayModeData *)mode->driverdata;
[data->uiscreenmode release];
SDL_free(data);
mode->driverdata = NULL;
}
}

void
UIKit_VideoQuit(_THIS)
{
Expand All @@ -366,11 +369,10 @@ static void UIKit_DeleteDevice(SDL_VideoDevice * device)
[data->uiscreen release];
SDL_free(data);
display->driverdata = NULL;
UIKit_ReleaseUIScreenMode(&display->desktop_mode);
UIKit_ReleaseUIScreenMode(&display->current_mode);
UIKit_FreeDisplayModeData(&display->desktop_mode);
for (j = 0; j < display->num_display_modes; j++) {
SDL_DisplayMode *mode = &display->display_modes[j];
UIKit_ReleaseUIScreenMode(mode);
UIKit_FreeDisplayModeData(mode);
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/video/uikit/SDL_uikitwindow.m
View file
Expand Up @@ -180,9 +180,7 @@ static int SetupWindowData(_THIS, SDL_Window *window, UIWindow *uiwindow, SDL_bo
// desktop_mode doesn't change here (the higher level will
// use it to set all the screens back to their defaults
// upon window destruction, SDL_Quit(), etc.
[((UIScreenMode *) display->current_mode.driverdata) release];
display->current_mode = *bestmode;
[((UIScreenMode *) display->current_mode.driverdata) retain];
}
}
}
Expand Down

0 comments on commit 5d2fff5

Please sign in to comment.