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 29, 2021
1 parent eeeda74 commit 333475a
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 102 deletions.
2 changes: 1 addition & 1 deletion src/SFML/Window/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ elseif(SFML_OS_MACOSX)
${SRCROOT}/OSX/WindowImplCocoa.hpp
${SRCROOT}/OSX/WindowImplCocoa.mm
${SRCROOT}/OSX/WindowImplDelegateProtocol.h
${SRCROOT}/OSX/AutoreleasePoolWrapper.h
${SRCROOT}/OSX/AutoreleasePoolWrapper.hpp
${SRCROOT}/OSX/AutoreleasePoolWrapper.mm
)
source_group("mac" FILES ${PLATFORM_SRC})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,54 @@
//
////////////////////////////////////////////////////////////

#ifndef SFML_AUTORELEASEPOOL_HPP
#define SFML_AUTORELEASEPOOL_HPP

////////////////////////////////////////////////////////////
/// \brief Ensure one autorelease pool is available on this thread
///
/// Predefine OBJ-C classes
////////////////////////////////////////////////////////////
void ensureThreadHasPool(void);
#ifdef __OBJC__

@class NSAutoreleasePool;
typedef NSAutoreleasePool* NSAutoreleasePoolRef;

#else // If C++

typedef void* NSAutoreleasePoolRef;

#endif

namespace sf
{

////////////////////////////////////////////////////////////
/// \brief Drain the thread's pool but keep it alive
/// \brief Wraps an NSAutoreleasePool that is created when the object is
/// constructed and is drained when the object is destroyed.
///
////////////////////////////////////////////////////////////
void drainThreadPool(void);
class AutoreleasePool
{
public:
////////////////////////////////////////////////////////////
/// \brief Construct a new NSAutoreleasePool.
///
////////////////////////////////////////////////////////////
AutoreleasePool();

////////////////////////////////////////////////////////////
/// \brief Destructor. Drains the autorelease pool.
///
////////////////////////////////////////////////////////////
~AutoreleasePool();

private:

////////////////////////////////////////////////////////////
// Member data
////////////////////////////////////////////////////////////
NSAutoreleasePoolRef pool; ///< The autorelease pool.
};

} // namespace sf

#endif // SFML_AUTORELEASEPOOL_HPP
75 changes: 10 additions & 65 deletions src/SFML/Window/OSX/AutoreleasePoolWrapper.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,80 +26,25 @@
////////////////////////////////////////////////////////////
// Headers
////////////////////////////////////////////////////////////
#include <cassert>
#include <pthread.h>

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


////////////////////////////////////////////////////////////
/// Here we manage one and only one pool by thread. This prevents draining one
/// pool and making other pools invalid which can lead to a crash on 10.5 and an
/// annoying message on 10.6 (*** attempt to pop an unknown autorelease pool).
///
////////////////////////////////////////////////////////////


////////////////////////////////////////////////////////////
// Private data
////////////////////////////////////////////////////////////
static pthread_key_t poolKey;
static pthread_once_t initOnceToken = PTHREAD_ONCE_INIT;


////////////////////////////////////////////////////////////
/// \brief (local function) Drain one more time the pool
/// but this time don't create a new one.
///
////////////////////////////////////////////////////////////
static void destroyPool(void* data)
{
NSAutoreleasePool* pool = (NSAutoreleasePool*)data;
[pool drain];
}
#include <SFML/Window/OSX/AutoreleasePoolWrapper.hpp>

#import <Foundation/Foundation.h>

////////////////////////////////////////////////////////////
/// \brief (local function) Init the pthread key for the pool
///
////////////////////////////////////////////////////////////
static void createPoolKey(void)
namespace sf
{
pthread_key_create(&poolKey, destroyPool);
}


////////////////////////////////////////////////////////////
/// \brief (local function) Store a new pool for this thread
///
////////////////////////////////////////////////////////////
static void createNewPool(void)
////////////////////////////////////////////////////////
AutoreleasePool::AutoreleasePool()
{
NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
pthread_setspecific(poolKey, pool);
pool = [[NSAutoreleasePool alloc] init];
}


////////////////////////////////////////////////////////////
void ensureThreadHasPool(void)
////////////////////////////////////////////////////////
AutoreleasePool::~AutoreleasePool()
{
pthread_once(&initOnceToken, createPoolKey);
if (pthread_getspecific(poolKey) == NULL)
{
createNewPool();
}
}


////////////////////////////////////////////////////////////
void drainThreadPool(void)
{
void* data = pthread_getspecific(poolKey);
assert(data != NULL);

// Drain the pool but keep it alive by creating a new one
destroyPool(data);
createNewPool();
[pool drain];
}

} // namespace sf
3 changes: 3 additions & 0 deletions src/SFML/Window/OSX/ClipboardImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
////////////////////////////////////////////////////////////
// Headers
////////////////////////////////////////////////////////////
#include <SFML/Window/OSX/AutoreleasePoolWrapper.hpp>
#include <SFML/Window/OSX/ClipboardImpl.hpp>

#import <AppKit/AppKit.h>
Expand All @@ -37,6 +38,7 @@
////////////////////////////////////////////////////////////
String ClipboardImpl::getString()
{
AutoreleasePool pool;
NSPasteboard* pboard = [NSPasteboard generalPasteboard];
NSString* data = [pboard stringForType:NSPasteboardTypeString];

Expand All @@ -50,6 +52,7 @@
////////////////////////////////////////////////////////////
void ClipboardImpl::setString(const String& text)
{
AutoreleasePool pool;
std::basic_string<Uint8> utf8 = text.toUtf8();
NSString* data = [[NSString alloc] initWithBytes:utf8.data()
length:utf8.length()
Expand Down
12 changes: 5 additions & 7 deletions src/SFML/Window/OSX/CursorImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
// Headers
////////////////////////////////////////////////////////////
#include <SFML/Window/OSX/CursorImpl.hpp>
#include <SFML/Window/OSX/AutoreleasePoolWrapper.hpp>

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

Expand All @@ -53,24 +53,21 @@
{

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


////////////////////////////////////////////////////////////
CursorImpl::~CursorImpl()
{
AutoreleasePool pool;
[m_cursor release];
}


////////////////////////////////////////////////////////////
bool CursorImpl::loadFromPixels(const Uint8* pixels, Vector2u size, Vector2u hotspot)
{
AutoreleasePool pool;
if (m_cursor)
{
[m_cursor release];
Expand All @@ -89,6 +86,7 @@
////////////////////////////////////////////////////////////
bool CursorImpl::loadFromSystem(Cursor::Type type)
{
AutoreleasePool pool;
NSCursor* newCursor = nil;

switch (type)
Expand Down
7 changes: 7 additions & 0 deletions src/SFML/Window/OSX/InputImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
////////////////////////////////////////////////////////////
#include <SFML/Window/VideoMode.hpp>
#include <SFML/Window/Window.hpp>
#include <SFML/Window/OSX/AutoreleasePoolWrapper.hpp>
#include <SFML/Window/OSX/InputImpl.hpp>
#include <SFML/Window/OSX/HIDInputManager.hpp>
#include <SFML/System/Err.hpp>
Expand Down Expand Up @@ -125,6 +126,7 @@
////////////////////////////////////////////////////////////
bool InputImpl::isKeyPressed(Keyboard::Key key)
{
AutoreleasePool pool;
return HIDInputManager::getInstance().isKeyPressed(key);
}

Expand All @@ -139,6 +141,7 @@
////////////////////////////////////////////////////////////
bool InputImpl::isMouseButtonPressed(Mouse::Button button)
{
AutoreleasePool pool;
NSUInteger state = [NSEvent pressedMouseButtons];
NSUInteger flag = 1 << button;
return (state & flag) != 0;
Expand All @@ -148,6 +151,7 @@
////////////////////////////////////////////////////////////
Vector2i InputImpl::getMousePosition()
{
AutoreleasePool pool;
// Reverse Y axis to match SFML coord.
NSPoint pos = [NSEvent mouseLocation];
pos.y = sf::VideoMode::getDesktopMode().height - pos.y;
Expand All @@ -160,6 +164,7 @@
////////////////////////////////////////////////////////////
Vector2i InputImpl::getMousePosition(const WindowBase& relativeTo)
{
AutoreleasePool pool;
SFOpenGLView* view = getSFOpenGLViewFromSFMLWindow(relativeTo);

// No view ?
Expand All @@ -177,6 +182,7 @@
////////////////////////////////////////////////////////////
void InputImpl::setMousePosition(const Vector2i& position)
{
AutoreleasePool pool;
// Here we don't need to reverse the coordinates.
int scale = [[NSScreen mainScreen] backingScaleFactor];
CGPoint pos = CGPointMake(position.x / scale, position.y / scale);
Expand All @@ -195,6 +201,7 @@
////////////////////////////////////////////////////////////
void InputImpl::setMousePosition(const Vector2i& position, const WindowBase& relativeTo)
{
AutoreleasePool pool;
SFOpenGLView* view = getSFOpenGLViewFromSFMLWindow(relativeTo);

// No view ?
Expand Down
7 changes: 7 additions & 0 deletions src/SFML/Window/OSX/JoystickImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
// Headers
////////////////////////////////////////////////////////////
#include <SFML/Window/JoystickImpl.hpp>
#include <SFML/Window/OSX/AutoreleasePoolWrapper.hpp>
#include <SFML/Window/OSX/HIDInputManager.hpp>
#include <SFML/Window/OSX/HIDJoystickManager.hpp>
#include <SFML/System/Err.hpp>
Expand Down Expand Up @@ -106,6 +107,7 @@ void JoystickImpl::cleanup()
////////////////////////////////////////////////////////////
bool JoystickImpl::isConnected(unsigned int index)
{
AutoreleasePool pool;
bool state = false; // Is the index-th joystick connected?

// First, let's check if the device was previously detected:
Expand Down Expand Up @@ -180,6 +182,7 @@ bool JoystickImpl::isConnected(unsigned int index)
////////////////////////////////////////////////////////////
bool JoystickImpl::open(unsigned int index)
{
AutoreleasePool pool;
m_index = index;
m_hat = NULL;
Location deviceLoc = m_locationIDs[index]; // The device we need to load
Expand Down Expand Up @@ -325,6 +328,7 @@ bool JoystickImpl::open(unsigned int index)
////////////////////////////////////////////////////////////
void JoystickImpl::close()
{
AutoreleasePool pool;
for (ButtonsVector::iterator it(m_buttons.begin()); it != m_buttons.end(); ++it)
CFRelease(*it);
m_buttons.clear();
Expand All @@ -345,6 +349,7 @@ void JoystickImpl::close()
////////////////////////////////////////////////////////////
JoystickCaps JoystickImpl::getCapabilities() const
{
AutoreleasePool pool;
JoystickCaps caps;

// Buttons:
Expand All @@ -364,13 +369,15 @@ JoystickCaps JoystickImpl::getCapabilities() const
////////////////////////////////////////////////////////////
Joystick::Identification JoystickImpl::getIdentification() const
{
AutoreleasePool pool;
return m_identification;
}


////////////////////////////////////////////////////////////
JoystickState JoystickImpl::update()
{
AutoreleasePool pool;
static const JoystickState disconnectedState; // return this if joystick was disconnected
JoystickState state; // otherwise return that
state.connected = true;
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

0 comments on commit 333475a

Please sign in to comment.