From 5020a810dbbfcd6146c0676099c30a7cee5654ac Mon Sep 17 00:00:00 2001 From: Stefan Csomor Date: Sun, 21 Apr 2019 23:52:37 +0200 Subject: [PATCH] Fix macOS memory leaks, also avoid false positive warnings from clang analyzer __clang_analyzer__ is a constant that only is defined during analyze build, this helps avoiding false positives as long as there is no specific way to silence analyzer messages --- include/wx/osx/core/private.h | 77 +++++++++++++++++++++++++++++++++ include/wx/osx/webview_webkit.h | 6 ++- src/osx/carbon/utilscocoa.mm | 66 ++++++++++++++-------------- src/osx/cocoa/menu.mm | 1 + src/osx/cocoa/window.mm | 5 ++- src/osx/webview_webkit.mm | 34 ++++++++++----- src/stc/PlatWXcocoa.mm | 1 + 7 files changed, 143 insertions(+), 47 deletions(-) diff --git a/include/wx/osx/core/private.h b/include/wx/osx/core/private.h index dfe47f4efd..bfdd0abb2a 100644 --- a/include/wx/osx/core/private.h +++ b/include/wx/osx/core/private.h @@ -1013,6 +1013,83 @@ void wxMacCocoaRelease( void* obj ); void wxMacCocoaAutorelease( void* obj ); void* wxMacCocoaRetain( void* obj ); +// shared_ptr like API for NSObject and subclasses +template +class wxNSObjRef +{ +public: + typedef T element_type; + + wxNSObjRef() + : m_ptr(NULL) + { + } + + wxNSObjRef( T p ) + : m_ptr(p) + { + } + + wxNSObjRef( const wxNSObjRef& otherRef ) + : m_ptr(wxMacCocoaRetain(otherRef.m_ptr)) + { + } + + wxNSObjRef& operator=( const wxNSObjRef& otherRef ) + { + if (this != &otherRef) + { + wxMacCocoaRetain(otherRef.m_ptr); + wxMacCocoaRelease(m_ptr); + m_ptr = otherRef.m_ptr; + } + return *this; + } + + wxNSObjRef& operator=( T ptr ) + { + if (get() != ptr) + { + wxMacCocoaRetain(ptr); + wxMacCocoaRelease(m_ptr); + m_ptr = ptr; + } + return *this; + } + + + T get() const + { + return m_ptr; + } + + operator T() const + { + return m_ptr; + } + + T operator->() const + { + return m_ptr; + } + + void reset( T p = NULL ) + { + wxMacCocoaRelease(m_ptr); + m_ptr = p; // Automatic conversion should occur + } + + // Release the pointer, i.e. give up its ownership. + T release() + { + T p = m_ptr; + m_ptr = NULL; + return p; + } + +protected: + T m_ptr; +}; #endif // _WX_PRIVATE_CORE_H_ diff --git a/include/wx/osx/webview_webkit.h b/include/wx/osx/webview_webkit.h index 32730fb613..7e43bc37fa 100644 --- a/include/wx/osx/webview_webkit.h +++ b/include/wx/osx/webview_webkit.h @@ -160,11 +160,13 @@ private: OSXWebViewPtr m_webView; + WX_NSObject m_loadDelegate; + WX_NSObject m_policyDelegate; + WX_NSObject m_UIDelegate; + // we may use this later to setup our own mouse events, // so leave it in for now. void* m_webKitCtrlEventHandler; - //It should be WebView*, but WebView is an Objective-C class - //TODO: look into using DECLARE_WXCOCOA_OBJC_CLASS rather than this. }; class WXDLLIMPEXP_WEBVIEW wxWebViewFactoryWebKit : public wxWebViewFactory diff --git a/src/osx/carbon/utilscocoa.mm b/src/osx/carbon/utilscocoa.mm index 28de1bdecd..c8df9341a8 100644 --- a/src/osx/carbon/utilscocoa.mm +++ b/src/osx/carbon/utilscocoa.mm @@ -374,17 +374,17 @@ CGImageRef wxOSXCreateCGImageFromImage( WXImage nsimage, double *scaleptr ) #if wxOSX_USE_COCOA -static NSCursor* wxGetStockCursor( short sIndex ) +static NSCursor* wxCreateStockCursor( short sIndex ) { - ClassicCursor* pCursor = &gMacCursors[sIndex]; - //Classic mac cursors are 1bps 16x16 black and white with a //identical mask that is 1 for on and 0 for off - NSImage *theImage = [[NSImage alloc] initWithSize:NSMakeSize(16.0,16.0)]; + ClassicCursor* pCursor = &gMacCursors[sIndex]; + + wxNSObjRef theImage( [[NSImage alloc] initWithSize:NSMakeSize(16.0,16.0)] ); //NSCursor takes an NSImage takes a number of Representations - here //we need only one for the raw data - NSBitmapImageRep *theRep = [[NSBitmapImageRep alloc] + wxNSObjRef theRep( [[NSBitmapImageRep alloc] initWithBitmapDataPlanes: NULL // Tell Cocoa to allocate the planes for us. pixelsWide: 16 // All classic cursors are 16x16 pixelsHigh: 16 @@ -394,16 +394,16 @@ static NSCursor* wxGetStockCursor( short sIndex ) isPlanar: YES // Use a separate array for each sample colorSpaceName: NSCalibratedWhiteColorSpace // 0.0=black 1.0=white bytesPerRow: 2 // Rows in each plane are on 2-byte boundaries (no pad) - bitsPerPixel: 1]; // same as bitsPerSample since data is planar + bitsPerPixel: 1] ); // same as bitsPerSample since data is planar // Ensure that Cocoa allocated 2 and only 2 of the 5 possible planes unsigned char *planes[5]; [theRep getBitmapDataPlanes:planes]; - wxASSERT(planes[0] != NULL); - wxASSERT(planes[1] != NULL); - wxASSERT(planes[2] == NULL); - wxASSERT(planes[3] == NULL); - wxASSERT(planes[4] == NULL); + wxCHECK(planes[0] != NULL, nil); + wxCHECK(planes[1] != NULL, nil); + wxCHECK(planes[2] == NULL, nil); + wxCHECK(planes[3] == NULL, nil); + wxCHECK(planes[4] == NULL, nil); // NOTE1: The Cursor's bits field is white=0 black=1.. thus the bitwise-not // Why not use NSCalibratedBlackColorSpace? Because that reverses the @@ -437,10 +437,6 @@ static NSCursor* wxGetStockCursor( short sIndex ) hotSpot:NSMakePoint(pCursor->hotspot[1], pCursor->hotspot[0]) ]; - //do the usual cleanups - [theRep release]; - [theImage release]; - //return the new cursor return theCursor; } @@ -460,7 +456,7 @@ WX_NSCursor wxMacCocoaCreateStockCursor( int cursor_type ) // according to the HIG // cursor = [[NSCursor arrowCursor] retain]; // but for crossplatform compatibility we display a watch cursor - cursor = wxGetStockCursor(kwxCursorWatch); + cursor = wxCreateStockCursor(kwxCursorWatch); break; case wxCURSOR_IBEAM: @@ -472,11 +468,11 @@ WX_NSCursor wxMacCocoaCreateStockCursor( int cursor_type ) break; case wxCURSOR_SIZENWSE: - cursor = wxGetStockCursor(kwxCursorSizeNWSE); + cursor = wxCreateStockCursor(kwxCursorSizeNWSE); break; case wxCURSOR_SIZENESW: - cursor = wxGetStockCursor(kwxCursorSizeNESW); + cursor = wxCreateStockCursor(kwxCursorSizeNESW); break; case wxCURSOR_SIZEWE: @@ -488,7 +484,7 @@ WX_NSCursor wxMacCocoaCreateStockCursor( int cursor_type ) break; case wxCURSOR_SIZING: - cursor = wxGetStockCursor(kwxCursorSize); + cursor = wxCreateStockCursor(kwxCursorSize); break; case wxCURSOR_HAND: @@ -496,47 +492,47 @@ WX_NSCursor wxMacCocoaCreateStockCursor( int cursor_type ) break; case wxCURSOR_BULLSEYE: - cursor = wxGetStockCursor(kwxCursorBullseye); + cursor = wxCreateStockCursor(kwxCursorBullseye); break; case wxCURSOR_PENCIL: - cursor = wxGetStockCursor(kwxCursorPencil); + cursor = wxCreateStockCursor(kwxCursorPencil); break; case wxCURSOR_MAGNIFIER: - cursor = wxGetStockCursor(kwxCursorMagnifier); + cursor = wxCreateStockCursor(kwxCursorMagnifier); break; case wxCURSOR_NO_ENTRY: - cursor = wxGetStockCursor(kwxCursorNoEntry); + cursor = wxCreateStockCursor(kwxCursorNoEntry); break; case wxCURSOR_PAINT_BRUSH: - cursor = wxGetStockCursor(kwxCursorPaintBrush); + cursor = wxCreateStockCursor(kwxCursorPaintBrush); break; case wxCURSOR_POINT_LEFT: - cursor = wxGetStockCursor(kwxCursorPointLeft); + cursor = wxCreateStockCursor(kwxCursorPointLeft); break; case wxCURSOR_POINT_RIGHT: - cursor = wxGetStockCursor(kwxCursorPointRight); + cursor = wxCreateStockCursor(kwxCursorPointRight); break; case wxCURSOR_QUESTION_ARROW: - cursor = wxGetStockCursor(kwxCursorQuestionArrow); + cursor = wxCreateStockCursor(kwxCursorQuestionArrow); break; case wxCURSOR_BLANK: - cursor = wxGetStockCursor(kwxCursorBlank); + cursor = wxCreateStockCursor(kwxCursorBlank); break; case wxCURSOR_RIGHT_ARROW: - cursor = wxGetStockCursor(kwxCursorRightArrow); + cursor = wxCreateStockCursor(kwxCursorRightArrow); break; case wxCURSOR_SPRAYCAN: - cursor = wxGetStockCursor(kwxCursorRoller); + cursor = wxCreateStockCursor(kwxCursorRoller); break; case wxCURSOR_OPEN_HAND: @@ -547,15 +543,21 @@ WX_NSCursor wxMacCocoaCreateStockCursor( int cursor_type ) cursor = [[NSCursor closedHandCursor] retain]; break; - case wxCURSOR_CHAR: case wxCURSOR_ARROW: + cursor = [[NSCursor arrowCursor] retain]; + break; + + case wxCURSOR_CHAR: case wxCURSOR_LEFT_BUTTON: case wxCURSOR_RIGHT_BUTTON: case wxCURSOR_MIDDLE_BUTTON: default: - cursor = [[NSCursor arrowCursor] retain]; break; } + + if ( cursor == nil ) + cursor = [[NSCursor arrowCursor] retain]; + return cursor; } diff --git a/src/osx/cocoa/menu.mm b/src/osx/cocoa/menu.mm index b2943a9828..8c54ad4a53 100644 --- a/src/osx/cocoa/menu.mm +++ b/src/osx/cocoa/menu.mm @@ -254,6 +254,7 @@ public : [windowMenuItem setSubmenu:windowMenu]; [windowMenu release]; [m_osxMenu addItem:windowMenuItem]; + [windowMenuItem release]; } return windowMenu; } diff --git a/src/osx/cocoa/window.mm b/src/osx/cocoa/window.mm index 3eda6351c0..6f570d531e 100644 --- a/src/osx/cocoa/window.mm +++ b/src/osx/cocoa/window.mm @@ -3563,12 +3563,15 @@ bool wxWidgetCocoaImpl::EnableTouchEvents(int eventsMask) } else // We do want to have gesture events. { + // clang does not see that the owning object always destroys its extra field +#ifndef __clang_analyzer__ wxCocoaGestures::StoreForObject ( this, new wxCocoaGesturesImpl(this, m_osxView, eventsMask) ); - +#endif + [m_osxView setAcceptsTouchEvents:YES]; } diff --git a/src/osx/webview_webkit.mm b/src/osx/webview_webkit.mm index be3f646872..f818b090f5 100644 --- a/src/osx/webview_webkit.mm +++ b/src/osx/webview_webkit.mm @@ -137,17 +137,23 @@ bool wxWebViewWebKit::Create(wxWindow *parent, [[WebViewLoadDelegate alloc] initWithWxWindow: this]; [m_webView setFrameLoadDelegate:loadDelegate]; + + m_loadDelegate = loadDelegate; // this is used to veto page loads, etc. WebViewPolicyDelegate* policyDelegate = [[WebViewPolicyDelegate alloc] initWithWxWindow: this]; [m_webView setPolicyDelegate:policyDelegate]; + + m_policyDelegate = policyDelegate; WebViewUIDelegate* uiDelegate = [[WebViewUIDelegate alloc] initWithWxWindow: this]; [m_webView setUIDelegate:uiDelegate]; + + m_UIDelegate = uiDelegate; #endif //Register our own class for custom scheme handling [NSURLProtocol registerClass:[WebViewCustomProtocol class]]; @@ -160,21 +166,13 @@ wxWebViewWebKit::~wxWebViewWebKit() { #if wxOSX_USE_IPHONE #else - WebViewLoadDelegate* loadDelegate = [m_webView frameLoadDelegate]; - WebViewPolicyDelegate* policyDelegate = [m_webView policyDelegate]; - WebViewUIDelegate* uiDelegate = [m_webView UIDelegate]; [m_webView setFrameLoadDelegate: nil]; [m_webView setPolicyDelegate: nil]; [m_webView setUIDelegate: nil]; - if (loadDelegate) - [loadDelegate release]; - - if (policyDelegate) - [policyDelegate release]; - - if (uiDelegate) - [uiDelegate release]; + [m_loadDelegate release]; + [m_policyDelegate release]; + [m_UIDelegate release]; #endif } @@ -1033,8 +1031,16 @@ wxString nsErrorToWxHtmlError(NSError* error, wxWebViewNavigationError* out) wxString wxpath = wxCFStringRef::AsString(path); wxString scheme = wxCFStringRef::AsString([[request URL] scheme]); + + // since canInitRequest has already checked whether this scheme is supported + // the hash map contains this entry, but to satisfy static code analysis + // suspecting nullptr dereference ... +#ifndef __clang_analyzer__ wxFSFile* file = g_stringHandlerMap[scheme]->GetFile(wxpath); - +#else + wxFSFile* file = NULL; +#endif + if (!file) { NSError *error = [[NSError alloc] initWithDomain:NSURLErrorDomain @@ -1042,6 +1048,8 @@ wxString nsErrorToWxHtmlError(NSError* error, wxWebViewNavigationError* out) userInfo:nil]; [client URLProtocol:self didFailWithError:error]; + + [error release]; return; } @@ -1069,6 +1077,8 @@ wxString nsErrorToWxHtmlError(NSError* error, wxWebViewNavigationError* out) //Notify that we have finished [client URLProtocolDidFinishLoading:self]; + [data release]; + [response release]; } diff --git a/src/stc/PlatWXcocoa.mm b/src/stc/PlatWXcocoa.mm index 7b798f954e..1c5498b2be 100644 --- a/src/stc/PlatWXcocoa.mm +++ b/src/stc/PlatWXcocoa.mm @@ -111,6 +111,7 @@ WX_NSWindow CreateFloatingWindow(wxWindow* wxWin) void CloseFloatingWindow(WX_NSWindow nsWin) { + [nsWin setReleasedWhenClosed:YES]; [nsWin close]; }