Bug 165809 – NSArray leaks seen in Safari, allocated under WKIconDatabaseTryCopyCGImageArrayForURL

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Joseph Pecoraro]

Summary:
NSArray leaks seen in WebContentProcess, allocated under WKIconDatabaseTryCopyCGImageArrayForURL.

Leak: 0x7f9d5c50e570  size=32  zone: DefaultMallocZone_0x108b79000   NSArray (Object Storage)  C  CoreFoundation
	0x627c73f0 0x00007f9d 0x627df480 0x00007f9d 	.s|b......}b....
	0x627ea4e0 0x00007f9d 0x6402d690 0x00007f9d 	..~b.......d....
	Call stack: [thread 0x7fffdb8e73c0]: 
        | start 
        | NSApplicationMain 
        | -[NSApplication run] 
        | -[BrowserApplication nextEventMatchingMask:untilDate:inMode:dequeue:] 
        | -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 
        | _DPSNextEvent 
        | _BlockUntilNextEventMatchingListInModeWithFilter 
        | ReceiveNextEventCommon 
        | RunCurrentEventLoopInMode 
        | CFRunLoopRunSpecific 
        | __CFRunLoopRun 
        | __CFRunLoopDoSource1 
        | __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__ 
        | mshMIGPerform 
        | _XCopyAttributeValue 
        | _AXXMIGCopyAttributeValue 
        | CopyAttributeValue 
        | CopyCarbonUIElementAttributeValue 
        | CarbonCopyAttributeValueCallback(__CFData const*, unsigned int, __CFString const*, void const**, void*) 
        | HLTBCopyUIElementAttributeValue 
        | Accessible::GetNamedAttributeData(__CFString const*, void const*, void const**, unsigned char*) 
        | SendEventToEventTargetWithOptions 
        | SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) 
        | DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) 
        | HIObject::EventHook(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) 
        | HIObject::HandleClassAccessibilityEvent(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) 
        | HIObject::DispatchAccessibilityEvent(OpaqueEventRef*, unsigned long long, AccessibilityHandlers const*, void*) 
        | MenuData::GetNamedAccessibleAttributeSelf(unsigned long long, __CFString const*, unsigned int, OpaqueEventRef*) 
        | MenuData::HandleGetNamedAccessibleAttribute(unsigned long long, __CFString const*, unsigned int, OpaqueEventRef*) 
        | OpenMenuForInspection(MenuData*) 
        | _SimulateMenuOpening 
        | SendMenuOpening(MenuSelectData*, MenuData*, double, unsigned int, unsigned int, __CFDictionary*, unsigned char, unsigned char*) 
        | SendMenuPopulate(MenuData*, OpaqueEventTargetRef*, unsigned int, double, unsigned int, OpaqueEventRef*, unsigned char, unsigned char*) 
        | SendEventToEventTargetWithOptions 
        | SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) 
        | DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) 
        | NSSLMMenuEventHandler 
        | -[NSCarbonMenuImpl _carbonPopulateEvent:handlerCallRef:] 
        | -[NSMenu _populateWithEventRef:] 
        | -[NSMenu _populateFromDelegateWithEventRef:] 
        | -[HistoryBookmarkSource menuNeedsUpdate:] 
        | -[HistoryBookmarkSource _updateHistoryMenu] 
        | -[HistoryBookmarkSource _updateRecentlyClosedSubmenu] 
        | -[ClosedTabOrWindowMenuBuilder buildClosedTabOrWindowMenu] 
        | -[ClosedTabOrWindowMenuBuilder _appendToMenuUsingWindowPolicy:] 
        | -[ClosedTabOrWindowMenuBuilder _menuItemsForWindowItemPolicyExpandWindowsIntoIndentedTabs:] 
        | -[ClosedTabOrWindowMenuBuilder _menuItemForWindowItemPolicyExpandWindowsIntoIndentedTabsWithSingleNonDisposableTab:] 
        | -[ClosedTabOrWindowMenuBuilder _itemIconForURLString:] 
        | Safari::IconController::bestSiteIconNS(NSString*, CGSize const&, bool*) const 
        | Safari::IconController::bestSiteIconDataForURLString(NSString*, CGSize) const 
        | Safari::IconController::bestSiteIconForURLString(NSString*, CGSize const&) const 
        | Safari::IconController::bestFallbackCandidate(NSURL*, CGSize const&) const 
        | Safari::WK::IconDatabase::cgImageArrayForURL(Safari::WK::URL const&) const 
        | WKIconDatabaseTryCopyCGImageArrayForURL 
        | -[__NSArrayM insertObject:atIndex:] 
        | malloc

It looks like WKIconDatabaseTryCopyCGImageArrayForURL has an extra retain (it does both a Create + Retain).

[Joseph Pecoraro]

I believe this is a regression caused by:
https://trac.webkit.org/changeset/205682

[Joseph Pecoraro]

Created attachment 297030 [details]
[PATCH] Proposed Fix

Comment on attachment 297030 [details]
[PATCH] Proposed Fix

I would have switched this code over to RetainPtr, adopting the newly-created array at first and explicitly leaking it on return, but this is OK too.

[WebKit Commit Bot]

Comment on attachment 297030 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 297030

Committed r209769: <http://trac.webkit.org/changeset/209769>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.