Bug 165809 - NSArray leaks seen in Safari, allocated under WKIconDatabaseTryCopyCGImageArrayForURL
Summary: NSArray leaks seen in Safari, allocated under WKIconDatabaseTryCopyCGImageArr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-13 11:30 PST by Joseph Pecoraro
Modified: 2016-12-13 12:23 PST (History)
5 users (show)

See Also:

Attachments
[PATCH] Proposed Fix (1.26 KB, patch)
2016-12-13 11:45 PST, Joseph Pecoraro 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-12-13 11:30:00 PST 
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).
Comment 1 Joseph Pecoraro 2016-12-13 11:30:32 PST 
I believe this is a regression caused by:
https://trac.webkit.org/changeset/205682
Comment 2 Joseph Pecoraro 2016-12-13 11:45:16 PST 
Created attachment 297030 [details]
[PATCH] Proposed Fix
Comment 3 mitz 2016-12-13 11:56:37 PST 
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.
Comment 4 WebKit Commit Bot 2016-12-13 12:23:41 PST 
Comment on attachment 297030 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 297030

Committed r209769: <http://trac.webkit.org/changeset/209769>
Comment 5 WebKit Commit Bot 2016-12-13 12:23:45 PST 
All reviewed patches have been landed.  Closing bug.