Bug 145549

Summary: REGRESSION(r185091): Crash happens on indexdb tests
Product: WebKit Reporter: Gyuyoung Kim <gyuyoung.kim>
Component: New BugsAssignee: Gyuyoung Kim <gyuyoung.kim>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, commit-queue, darin, jsbell, ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 145508    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Gyuyoung Kim
Reported 2015-06-01 22:10:14 PDT
Some indexeddb tests come to crash since r185091. > Regressions: Unexpected crashes (5) > storage/indexeddb/error-causes-abort-by-default.html [ Crash ] > storage/indexeddb/exception-in-event-aborts.html [ Crash ] > storage/indexeddb/mozilla/add-twice-failure.html [ Crash ] > storage/indexeddb/request-event-propagation.html [ Crash ] > storage/indexeddb/transaction-event-propagation.html [ Crash ]
Attachments
Patch (10.99 KB, patch)
2015-06-01 23:35 PDT, Gyuyoung Kim 		no flags
DetailsFormatted DiffDiff
Patch (10.99 KB, patch)
2015-06-01 23:42 PDT, Gyuyoung Kim 		no flags
DetailsFormatted DiffDiff
Patch (12.73 KB, patch)
2015-06-03 21:50 PDT, Gyuyoung Kim 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Gyuyoung Kim
Comment 1 2015-06-01 23:35:54 PDT
Created attachment 254058 [details] Patch
Gyuyoung Kim
Comment 2 2015-06-01 23:42:34 PDT
Created attachment 254059 [details] Patch
Csaba Osztrogonác
Comment 3 2015-06-02 01:23:37 PDT
Comment on attachment 254059 [details] Patch rs=me
WebKit Commit Bot
Comment 4 2015-06-02 02:13:05 PDT
Comment on attachment 254059 [details] Patch Clearing flags on attachment: 254059 Committed r185107: <http://trac.webkit.org/changeset/185107>
WebKit Commit Bot
Comment 5 2015-06-02 02:13:10 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 6 2015-06-02 11:22:49 PDT
Comment on attachment 254059 [details] Patch This doesn’t look right. It just looks like a storage leak to me. Could you explain what code calls deref on these errors?
Darin Adler
Comment 7 2015-06-02 11:32:14 PDT
I am almost certain this patch is wrong. It’s possible adding a call to WTF::move() around the create functions will fix things; the trick is to change a temporary Ref into a RefPtr. I don’t really understand why, but I am quite sure that by calling leakRef we are actually leaking the objects, so I know that can’t be right. I don’t fully understand why the original crash was happening, though. Should have worked fine.
Gyuyoung Kim
Comment 8 2015-06-03 21:50:02 PDT
Reopening to attach new patch.
Gyuyoung Kim
Comment 9 2015-06-03 21:50:06 PDT
Created attachment 254244 [details] Patch
Gyuyoung Kim
Comment 10 2015-06-03 21:59:10 PDT
(In reply to comment #6) > Comment on attachment 254059 [details] > Patch > > This doesn’t look right. It just looks like a storage leak to me. Could you > explain what code calls deref on these errors? I guessed the deref was called by AsyncRequest::completeRequest() though, now I think I can't be 100% sure about it. http://trac.webkit.org/browser/trunk/Source/WebKit2/Shared/AsyncRequest.h#L101 > I am almost certain this patch is wrong. It’s possible adding a call to > WTF::move() around the create functions will fix things; the trick is to > change a temporary Ref into a RefPtr. I don’t really understand why, but I > am quite sure that by calling leakRef we are actually leaking the objects, > so I know that can’t be right. I don’t fully understand why the original > crash was happening, though. Should have worked fine. I agree that my previous fix can cause a storage leak because AsyncRequest::completeRequest() may not handle the pointer of IDBDatabaseError instance. However I would like to fix this problem with correct solution. So I upload a revert patch only for this issue. When I have correct solution for this issue, I will upload it again.
WebKit Commit Bot
Comment 11 2015-06-04 18:32:48 PDT
Comment on attachment 254244 [details] Patch Clearing flags on attachment: 254244 Committed r185233: <http://trac.webkit.org/changeset/185233>
WebKit Commit Bot
Comment 12 2015-06-04 18:32:54 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.