Skip to content

Commit

Permalink
Change handling of NSAutoreleasePool
Browse files Browse the repository at this point in the history
GUI toolkits such as wxWidgets and Qt provide their own NSAutoreleasePool
objects. To avoid pool corruption, it is required that the pools be nested,
which the previous implementation did not guarantee. Furthermore, autorelease
pools should always be drained in the same context (function, loop, etc.) that
they are created, which was not the case with the previous implementation
(https://developer.apple.com/documentation/foundation/nsautoreleasepool).

This commit complete removes the AutoreleasePoolWrapper, and functions that use
autorelease create and drain their own pools.

Should fix crashes in issue SFML#1549 and similar.
  • Loading branch information
jqdg committed Aug 24, 2021
1 parent eeeda74 commit a930938
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 175 deletions.
2 changes: 0 additions & 2 deletions src/SFML/Window/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,6 @@ elseif(SFML_OS_MACOSX)
${SRCROOT}/OSX/WindowImplCocoa.hpp
${SRCROOT}/OSX/WindowImplCocoa.mm
${SRCROOT}/OSX/WindowImplDelegateProtocol.h
${SRCROOT}/OSX/AutoreleasePoolWrapper.h
${SRCROOT}/OSX/AutoreleasePoolWrapper.mm
)
source_group("mac" FILES ${PLATFORM_SRC})
elseif(SFML_OS_IOS)
Expand Down
37 changes: 0 additions & 37 deletions src/SFML/Window/OSX/AutoreleasePoolWrapper.h

This file was deleted.

105 changes: 0 additions & 105 deletions src/SFML/Window/OSX/AutoreleasePoolWrapper.mm

This file was deleted.

8 changes: 1 addition & 7 deletions src/SFML/Window/OSX/CursorImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
////////////////////////////////////////////////////////////
#include <SFML/Window/OSX/CursorImpl.hpp>

#import <SFML/Window/OSX/AutoreleasePoolWrapper.h>
#import <SFML/Window/OSX/NSImage+raw.h>
#import <AppKit/AppKit.h>

Expand All @@ -53,12 +52,7 @@
{

////////////////////////////////////////////////////////////
CursorImpl::CursorImpl() :
m_cursor(nil)
{
// Just ask for a pool
ensureThreadHasPool();
}
CursorImpl::CursorImpl() : m_cursor(nil) {}


////////////////////////////////////////////////////////////
Expand Down
8 changes: 8 additions & 0 deletions src/SFML/Window/OSX/SFApplication.m
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ +(void)processEvent
////////////////////////////////////////////////////////
+(void)setUpMenuBar
{
NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];

[SFApplication sharedApplication]; // Make sure NSApp exists

// Set the main menu bar
Expand All @@ -76,6 +78,8 @@ +(void)setUpMenuBar
NSMenu* windowMenu = [[SFApplication newWindowMenu] autorelease];
[windowItem setSubmenu:windowMenu];
[NSApp setWindowsMenu:windowMenu];

[pool drain];
}


Expand All @@ -98,6 +102,8 @@ +(NSMenu*)newAppleMenu
// --------------------
// Quit AppName Command+Q

NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];

NSString* appName = [SFApplication applicationName];

// APPLE MENU
Expand Down Expand Up @@ -154,6 +160,8 @@ +(NSMenu*)newAppleMenu
action:@selector(terminate:)
keyEquivalent:@"q"];

[pool drain];

return appleMenu;
}

Expand Down
11 changes: 0 additions & 11 deletions src/SFML/Window/OSX/SFContext.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
#include <dlfcn.h>
#include <stdint.h>

#import <SFML/Window/OSX/AutoreleasePoolWrapper.h>

namespace sf
{
namespace priv
Expand All @@ -46,9 +44,6 @@
m_view(0),
m_window(0)
{
// Ask for a pool.
ensureThreadHasPool();

// Create the context
createContext(shared,
VideoMode::getDesktopMode().bitsPerPixel,
Expand All @@ -63,9 +58,6 @@
m_view(0),
m_window(0)
{
// Ask for a pool.
ensureThreadHasPool();

// Create the context.
createContext(shared, bitsPerPixel, settings);

Expand All @@ -85,9 +77,6 @@
// Ensure the process is setup in order to create a valid window.
WindowImplCocoa::setUpProcess();

// Ask for a pool.
ensureThreadHasPool();

// Create the context.
createContext(shared, VideoMode::getDesktopMode().bitsPerPixel, settings);

Expand Down
4 changes: 4 additions & 0 deletions src/SFML/Window/OSX/SFWindowController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ -(id)initWithMode:(const sf::VideoMode&)mode andStyle:(unsigned long)style
////////////////////////////////////////////////////////
-(void)setupFullscreenViewWithMode:(const sf::VideoMode&)mode
{
NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];

// Create a screen-sized window on the main display
sf::VideoMode desktop = sf::VideoMode::getDesktopMode();
sf::priv::scaleInWidthHeight(desktop, nil);
Expand Down Expand Up @@ -231,6 +233,8 @@ -(void)setupFullscreenViewWithMode:(const sf::VideoMode&)mode
// Populate the window and views
[masterView addSubview:m_oglView];
[m_window setContentView:masterView];

[pool drain];
}


Expand Down
17 changes: 4 additions & 13 deletions src/SFML/Window/OSX/WindowImplCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include <SFML/Window/OSX/WindowImplCocoa.hpp>
#include <SFML/System/Err.hpp>

#import <SFML/Window/OSX/AutoreleasePoolWrapper.h>
#import <SFML/Window/OSX/cpp_objc_conversion.h>
#import <SFML/Window/OSX/Scaling.h>
#import <SFML/Window/OSX/SFApplication.h>
Expand Down Expand Up @@ -84,9 +83,6 @@ void showMouseCursor()
WindowImplCocoa::WindowImplCocoa(WindowHandle handle) :
m_showCursor(true)
{
// Ask for a pool.
ensureThreadHasPool();

// Treat the handle as it real type
id nsHandle = (id)handle;
if ([nsHandle isKindOfClass:[NSWindow class]])
Expand Down Expand Up @@ -129,9 +125,6 @@ void showMouseCursor()
// Transform the app process.
setUpProcess();

// Ask for a pool.
ensureThreadHasPool();

// Use backing size
scaleInWidthHeight(mode, nil);

Expand All @@ -148,7 +141,10 @@ void showMouseCursor()
WindowImplCocoa::~WindowImplCocoa()
{
[m_delegate closeWindow];

// Tell the window/view controller (and the OpenGL view) that the delegate
// (this object) no longer exists to prevent events being sent to the window
// after it has been deleted.
[m_delegate setRequesterTo:0];
[m_delegate release];

// Put the next window in front, if any.
Expand All @@ -159,10 +155,6 @@ void showMouseCursor()
if ([nextWindow isVisible])
[nextWindow makeKeyAndOrderFront:nil];
}

drainThreadPool(); // Make sure everything was freed
// This solve some issue when sf::Window::Create is called for the
// second time (nothing was render until the function was called again)
}


Expand Down Expand Up @@ -399,7 +391,6 @@ void showMouseCursor()
void WindowImplCocoa::processEvents()
{
[m_delegate processEvent];
drainThreadPool(); // Reduce memory footprint
}

#pragma mark
Expand Down

0 comments on commit a930938

Please sign in to comment.