| Summary: | [QT] Possible Leaks WKRetainPtr<> should Adopt allocated Copy | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
| Component: | WebKit Qt | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | joepeck, psolanki, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
I should clarify. > WebKit2/UIProcess/API/qt/qweberror.cpp > 61: WKRetainPtr<WKStringRef> errorDomainPtr = adoptWK(WKErrorCopyDomain(d->error.get())); > 80: WKRetainPtr<WKURLRef> failingURL = adoptWK(WKErrorCopyFailingURL(d->error.get())); These look good because of the adoptWK(...). > WebKit2/UIProcess/qt/qwkhistory.cpp > 76: WKRetainPtr<WKStringRef> title = WKBackForwardListItemCopyTitle(d->m_backForwardListItem.get()); > 84: WKRetainPtr<WKURLRef> url = WKBackForwardListItemCopyURL(d->m_backForwardListItem.get()); These however don't look so good. Created attachment 103114 [details]
[PATCH] Untested Proposed Fix
I haven't tested this patch so I really suggest a WebKit2 / Qt developer take a look at this.
I did not do a Qt build to test this, I just inspected the source.
Comment on attachment 103114 [details] [PATCH] Untested Proposed Fix Clearing flags on attachment: 103114 Committed r92574: <http://trac.webkit.org/changeset/92574> All reviewed patches have been landed. Closing bug. |
shell> cd Source/ shell> a 'Retain.*?=.*?Copy' WebKit2/UIProcess/API/qt/qweberror.cpp 61: WKRetainPtr<WKStringRef> errorDomainPtr = adoptWK(WKErrorCopyDomain(d->error.get())); 80: WKRetainPtr<WKURLRef> failingURL = adoptWK(WKErrorCopyFailingURL(d->error.get())); WebKit2/UIProcess/qt/qwkhistory.cpp 76: WKRetainPtr<WKStringRef> title = WKBackForwardListItemCopyTitle(d->m_backForwardListItem.get()); 84: WKRetainPtr<WKURLRef> url = WKBackForwardListItemCopyURL(d->m_backForwardListItem.get()); These look like possible leaks. Since WKRetainPtr::operator= does a WKRetain and typically a WK*Copy*() will return a +1 allocation. I think the correct behavior here would be AdoptWK like: WebKit2/UIProcess/qt/qwkpreferences.cpp 93: WKRetainPtr<WKStringRef> stringRef(AdoptWK, WKPreferencesCopyStandardFontFamily(d->ref));