Bug 30167

Summary: [Qt] Memory leak caught in WebCore/platform/network/qt/QNetworkReplyHandler.cpp
Product: WebKit Reporter: Chang Shu <cshu>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: S60 Hardware   
OS: S60 3rd edition   
Attachments:
Description Flags
patch
cshu: review-
2nd patch
none
update ChangeLog none

Chang Shu
Reported 2009-10-07 07:59:19 PDT
Memory leak caught in WebCore/platform/network/qt/QNetworkReplyHandler.cpp. Function QNetworkReply::abort()
Attachments
patch (1.12 KB, patch)
2009-10-07 08:32 PDT, Chang Shu 		cshu: review-
DetailsFormatted DiffDiff
2nd patch (1.48 KB, patch)
2009-10-22 09:14 PDT, Chang Shu 		no flags
DetailsFormatted DiffDiff
update ChangeLog (1.66 KB, patch)
2009-10-22 11:29 PDT, Chang Shu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Chang Shu
Comment 1 2009-10-07 08:32:25 PDT
Created attachment 40790 [details] patch
Eric Seidel (no email)
Comment 2 2009-10-07 09:18:10 PDT
Comment on attachment 40790 [details] patch Wow. deleteLater() is a very scary memory management model. But OK I trust you. rubber-stamp=me.
Chang Shu
Comment 3 2009-10-08 08:17:40 PDT
(In reply to comment #2) > (From update of attachment 40790 [details]) > Wow. deleteLater() is a very scary memory management model. But OK I trust > you. rubber-stamp=me. Eric, Please set this to r- as it may cause problem. I am working on that. Sorry.
Chang Shu
Comment 4 2009-10-22 09:14:00 PDT
Created attachment 41662 [details] 2nd patch
Eric Seidel (no email)
Comment 5 2009-10-22 10:51:31 PDT
What sort of problems did the other patch cause? Can we test those problems with a unit test/layout test?
Chang Shu
Comment 6 2009-10-22 10:56:38 PDT
(In reply to comment #5) > What sort of problems did the other patch cause? Can we test those problems > with a unit test/layout test? If m_reply is NOT unhooked from QNetworkReplyHandler in release() function, it may reference QNetworkReplyHandler after the latter is deleted. This is the problem of the first patch. Not sure how to come up with a test case... It's related to download.
Eric Seidel (no email)
Comment 7 2009-10-22 10:58:55 PDT
Comment on attachment 41662 [details] 2nd patch Ideally your ChangeLog would give explanations like this, and make some small statement about why this change is untestable.
Chang Shu
Comment 8 2009-10-22 11:29:37 PDT
Created attachment 41670 [details] update ChangeLog
Eric Seidel (no email)
Comment 9 2009-10-22 11:33:15 PDT
Comment on attachment 41670 [details] update ChangeLog OK. I'm not a Qt expert, but as far as I can tell this looks sane.
WebKit Commit Bot
Comment 10 2009-10-22 12:56:16 PDT
Comment on attachment 41670 [details] update ChangeLog Clearing flags on attachment: 41670 Committed r49951: <http://trac.webkit.org/changeset/49951>
WebKit Commit Bot
Comment 11 2009-10-22 12:56:20 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.