From 1de495fe55d5b470db62eb7d6fddea4de39e9366 Mon Sep 17 00:00:00 2001 From: Mark Liversedge Date: Sat, 26 Jan 2013 12:36:23 +0000 Subject: [PATCH] Cocoa Memory Management Fixups A number of memory management errors fixed, no doubt there are more lurking in there. * CocoaInitializer now sets up an application wide NSAutoreleasePool * Kickr now init/releases an NSAutoreleasePool for its thread * Local autorelease pools in constructors etc have been removed * Searchbox and Button no longer release in the constructor The icon corruption seems to be improved (I think it is a memory management issue) but has not been totally removed. --- src/Kickr.cpp | 17 +++++++++++++++-- src/Kickr.h | 2 ++ src/MainWindow.cpp | 16 +++++++++++----- src/MainWindow.h | 2 ++ src/QtMacButton.mm | 6 +----- src/QtMacPopUpButton.mm | 3 --- src/QtMacSearchBox.mm | 4 ---- src/QtMacSegmentedButton.h | 38 +++++++++++++------------------------ src/QtMacSegmentedButton.mm | 13 ++----------- src/WFApi.h | 9 +++++++++ src/WFApi.mm | 23 ++++++++++++++-------- 11 files changed, 70 insertions(+), 63 deletions(-) diff --git a/src/Kickr.cpp b/src/Kickr.cpp index 30bec815d..fa7cbbe09 100644 --- a/src/Kickr.cpp +++ b/src/Kickr.cpp @@ -223,8 +223,14 @@ Kickr::find() int Kickr::connectKickr() { + // get a pool for this thread + pool = WFApi::getInstance()->getPool(); + // do we even have BTLE hardware? - if (WFApi::getInstance()->isBTLEEnabled() == false) return (-1); + if (WFApi::getInstance()->isBTLEEnabled() == false) { + WFApi::getInstance()->freePool(pool); + return (-1); + } // discover first... if (scanned == false) find(); @@ -239,7 +245,10 @@ Kickr::connectKickr() break; } } - if (found == false) return -1; + if (found == false) { + WFApi::getInstance()->freePool(pool); + return (-1); + } w->connectDevice(i); return 0; @@ -251,6 +260,10 @@ Kickr::disconnectKickr() // disconnect WFApi::getInstance()->disconnectDevice(); connected = false; + + // clear that pool now we're done + WFApi::getInstance()->freePool(pool); + return 0; } diff --git a/src/Kickr.h b/src/Kickr.h index 75b23f1d9..9a77b5f5e 100644 --- a/src/Kickr.h +++ b/src/Kickr.h @@ -85,6 +85,8 @@ private: QString deviceUUID; RealtimeData rt; + + void *pool; }; #endif // _GC_Kickr_h diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index e71b5be1b..af83a61cd 100644 --- a/src/MainWindow.cpp +++ b/src/MainWindow.cpp @@ -135,6 +135,11 @@ MainWindow::MainWindow(const QDir &home) : zones_(new Zones), hrzones_(new HrZones), ride(NULL), workout(NULL) { + #ifdef Q_OS_MAC + // get an autorelease pool setup + static CocoaInitializer cocoaInitializer; + #endif + #ifdef GC_HAVE_WFAPI WFApi *w = WFApi::getInstance(); // ensure created on main thread w->apiVersion();//shutup compiler @@ -204,7 +209,6 @@ MainWindow::MainWindow(const QDir &home) : #ifdef Q_OS_MAC // MAC NATIVE TOOLBAR - static CocoaInitializer cocoaInitializer; // we only need one setUnifiedTitleAndToolBarOnMac(true); head = addToolBar(cyclist); head->setContentsMargins(0,0,0,0); @@ -220,13 +224,15 @@ MainWindow::MainWindow(const QDir &home) : QHBoxLayout *lb = new QHBoxLayout(macAnalButtons); lb->setContentsMargins(0,0,0,0); lb->setSpacing(0); - QtMacButton *import = new QtMacButton(this, QtMacButton::TexturedRounded); - import->setImage(QPixmap(":images/mac/download.png")); + import = new QtMacButton(this, QtMacButton::TexturedRounded); + QPixmap importImg(":images/mac/download.png"); + import->setImage(importImg); import->setToolTip("Download"); lb->addWidget(import); lb->addWidget(new Spacer(this)); - QtMacButton *compose = new QtMacButton(this, QtMacButton::TexturedRounded); - compose->setImage(QPixmap(":images/mac/compose.png")); + compose = new QtMacButton(this, QtMacButton::TexturedRounded); + QPixmap composeImg(":images/mac/compose.png"); + compose->setImage(composeImg); compose->setToolTip("Create"); lb->addWidget(compose); diff --git a/src/MainWindow.h b/src/MainWindow.h index bcf8cb0d8..12cf9591b 100644 --- a/src/MainWindow.h +++ b/src/MainWindow.h @@ -67,6 +67,7 @@ class Lucene; class NamedSearches; class ChartSettings; class QtMacSegmentedButton; +class QtMacButton; class GcScopeBar; class RideFileCache; class Library; @@ -444,6 +445,7 @@ class MainWindow : public QMainWindow // Mac Native Support QWidget *toolBarWidgets; QWidget *macAnalButtons; + QtMacButton *import, *compose; QtMacSegmentedButton *styleSelector; QToolBar *head; GcScopeBar *scopebar; diff --git a/src/QtMacButton.mm b/src/QtMacButton.mm index 361b7a89f..7b011e314 100644 --- a/src/QtMacButton.mm +++ b/src/QtMacButton.mm @@ -27,7 +27,6 @@ static NSImage *fromQPixmap(const QPixmap &pixmap) NSImage *image = [[NSImage alloc] init]; [image addRepresentation:bitmapRep]; [image setTemplate:true]; - [bitmapRep release]; return image; } @@ -205,10 +204,7 @@ QtMacButton::QtMacButton(QWidget *parent, BezelStyle bezelStyle) : QWidget(paren [button setTarget:target]; [button setAction:@selector(clicked)]; - setupLayout(button, this); - - [button release]; } void QtMacButton::setWidth(int x) @@ -240,7 +236,7 @@ void QtMacButton::setImage(const QPixmap &image) Q_ASSERT(qtw); if (qtw) { [qtw->nsButton setImage:fromQPixmap(image)]; - //[qtw->nsButton setAlternateImage:fromQPixmap(image)]; + [qtw->nsButton setAlternateImage:fromQPixmap(image)]; [qtw->nsButton setButtonType:NSMomentaryPushButton]; } } diff --git a/src/QtMacPopUpButton.mm b/src/QtMacPopUpButton.mm index 0f0e97d0a..d11f36991 100644 --- a/src/QtMacPopUpButton.mm +++ b/src/QtMacPopUpButton.mm @@ -27,7 +27,6 @@ static NSImage *fromQPixmap(const QPixmap &pixmap) NSImage *image = [[NSImage alloc] init]; [image addRepresentation:bitmapRep]; [image setTemplate:true]; - [bitmapRep release]; return image; } @@ -191,8 +190,6 @@ QtMacPopUpButton::QtMacPopUpButton(QWidget *parent, BezelStyle bezelStyle) : QWi [button setAction:@selector(clicked)]; setupLayout(button, this); - - [button release]; } void QtMacPopUpButton::setToolTip(const QString &text) diff --git a/src/QtMacSearchBox.mm b/src/QtMacSearchBox.mm index 25c1b13e2..99e0bec7f 100644 --- a/src/QtMacSearchBox.mm +++ b/src/QtMacSearchBox.mm @@ -79,10 +79,6 @@ SearchWidget::SearchWidget(QtMacSearchBox *parent) //QMenu *qtMenu = createMenu(this); //NSMenu *nsMenu = qtMenu->macMenu(0); //[[search cell] setSearchMenuTemplate:nsMenu]; - - // Release our reference, since our super class takes ownership and we - // don't need it anymore. - [search release]; } SearchWidget::~SearchWidget() diff --git a/src/QtMacSegmentedButton.h b/src/QtMacSegmentedButton.h index 760644a35..0fde646ae 100644 --- a/src/QtMacSegmentedButton.h +++ b/src/QtMacSegmentedButton.h @@ -21,26 +21,8 @@ #include -//---------------------------------------------------------------------- -// Cocoa / OBJC helpers etc -// -// this is a utility -- since this source file -// is included in C++ and Objective-C code the -// declaration of native components is referenced -// directly or just as a void * -//---------------------------------------------------------------------- - -class CocoaInitializer -{ - public: - CocoaInitializer(); - ~CocoaInitializer(); - - private: - class Private; - Private* d; -}; - +// macros for compile time, depending if included in an obj-c +// or a c++ source file. Changes declaration of class types. #ifdef __OBJC__ # define ADD_COCOA_NATIVE_REF(CocoaClass) \ @class CocoaClass; \ @@ -49,12 +31,18 @@ class CocoaInitializer # define ADD_COCOA_NATIVE_REF(CocoaClass) typedef void *Native##CocoaClass##Ref #endif /* __OBJC__ */ -// The above is merely to do the following, but -// we may add more native widgets in the future -ADD_COCOA_NATIVE_REF (NSSegmentedControl); +ADD_COCOA_NATIVE_REF (NSAutoreleasePool); +class CocoaInitializer +{ + public: + CocoaInitializer(); + ~CocoaInitializer(); -// The native Cocoa segmented button is held within -// a QMacCocoaView container. + private: + NativeNSAutoreleasePoolRef pool; +}; + +ADD_COCOA_NATIVE_REF (NSSegmentedControl); class QtMacSegmentedButton : public QMacCocoaViewContainer { Q_OBJECT; diff --git a/src/QtMacSegmentedButton.mm b/src/QtMacSegmentedButton.mm index 7355b700e..0563ae14a 100644 --- a/src/QtMacSegmentedButton.mm +++ b/src/QtMacSegmentedButton.mm @@ -25,23 +25,15 @@ #import #import -/*---------------------------------------------------------------------- - * Utility functions - *----------------------------------------------------------------------*/ -class CocoaInitializer::Private -{ - public: -}; - CocoaInitializer::CocoaInitializer() { - d = new CocoaInitializer::Private(); + pool = [[NSAutoreleasePool alloc] init]; NSApplicationLoad(); } CocoaInitializer::~CocoaInitializer() { - delete d; + [pool release]; } @@ -56,7 +48,6 @@ static NSImage *fromQPixmap(const QPixmap &pixmap) NSImage *image = [[NSImage alloc] init]; [image addRepresentation:bitmapRep]; [image setTemplate:true]; - [bitmapRep release]; return image; } diff --git a/src/WFApi.h b/src/WFApi.h index 271bc44dc..3039ea664 100644 --- a/src/WFApi.h +++ b/src/WFApi.h @@ -85,6 +85,15 @@ public: void setSlope(double slope); void setLoad(int watts); + // NOTE: There is an application wide NSAutoreleasePool maintained + // in cocoa initialiser, but it is only to support activity on + // the main thread. + // The application code (e.g. Kickr.cpp) needs to get and free a + // pool for each thread, this is why we have a getPool/freePool + // method in WFApi, but never allocate a pool ourselves. + void *getPool(); + void freePool(void*); + signals: void currentStateChanged(int); // hardware conncector state changed int discoveredDevices(int,bool); diff --git a/src/WFApi.mm b/src/WFApi.mm index a9d7e7874..2e40dc49b 100644 --- a/src/WFApi.mm +++ b/src/WFApi.mm @@ -53,6 +53,7 @@ static QString toQString(const NSString *nsstr) // Objective C -- Private interface / Bridge to WF API classes //---------------------------------------------------------------------- + @interface WFBridge : NSObject { @public QPointer qtw; // the QT QObject public class @@ -82,9 +83,7 @@ static QString toQString(const NSString *nsstr) // By default BTLE is disabled -(BOOL)isBTLEEnabled { return [[WFHardwareConnector sharedConnector] isBTLEEnabled]; } -(BOOL)enableBTLE:(BOOL)bEnable inBondingMode:(BOOL)bBondingMode { - NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; bool result = [[WFHardwareConnector sharedConnector] enableBTLE:bEnable inBondingMode:bBondingMode]; - [pool drain]; return result; } @@ -105,10 +104,8 @@ static QString toQString(const NSString *nsstr) // scan -(BOOL)discoverDevicesOfType:(WFSensorType_t)eSensorType onNetwork:(WFNetworkType_t)eNetworkType searchTimeout:(NSTimeInterval)timeout { - NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; [discoveredSensors removeAllObjects]; [[WFHardwareConnector sharedConnector] discoverDevicesOfType:eSensorType onNetwork:eNetworkType searchTimeout:timeout]; //XXX ignoringreturn - [pool drain]; return true; } -(int)deviceCount { return [discoveredSensors count]; } @@ -120,7 +117,6 @@ static QString toQString(const NSString *nsstr) -(BOOL)connectDevice: (int)n { - //NSAutoreleasePool *pool = [[NSAutoreleasePool alloc]init]; // just in case there is a discovery in action, lets cancel it... [[WFHardwareConnector sharedConnector] cancelDiscoveryOnNetwork:WF_NETWORKTYPE_BTLE]; @@ -138,7 +134,6 @@ static QString toQString(const NSString *nsstr) // set delegate to receive connection status changes. self.sensorConnection.delegate = self; - //[pool drain]; return true; } @@ -218,6 +213,8 @@ static QString toQString(const NSString *nsstr) qtw->hasFirmwareUpdateAvalableForConnection(); //XXX do what? } +-(NSAutoreleasePool*) getPool { return [[NSAutoreleasePool alloc] init]; } +-(void) freePool:(NSAutoreleasePool*)pool { [pool release]; } @end //---------------------------------------------------------------------- @@ -229,10 +226,8 @@ WFApi *_gc_wfapi = NULL; // Construct the bridge to the WF API WFApi::WFApi() { - NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; wf = [[WFBridge alloc] init]; wf->qtw = this; - [pool drain]; } // Destroy the bridge to the WF API @@ -372,3 +367,15 @@ WFApi::getRealtimeData(RealtimeData *rt) rt->setCadence((int)[sd instantCadence]); rt->setWheelRpm((int)[sd instantWheelRPM]); } + +void * +WFApi::getPool() +{ + return (void*)[wf getPool]; +} + +void +WFApi::freePool(void *pool) +{ + [wf freePool:(NSAutoreleasePool*)pool]; +}