WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
51112
IDBTransactionBackedImpl instances can be accidentally deleted during calls to abort/commit.
https://bugs.webkit.org/show_bug.cgi?id=51112
Summary
IDBTransactionBackedImpl instances can be accidentally deleted during calls t...
Andrei Popescu
Reported
2010-12-15 08:45:12 PST
IDBTransactionBackedImpl instances can be accidentally deleted during calls to abort/commit. This happens because IDBTransactionBackedImpl invokes IDBTransactionCoordinator::didFinishTransaction(), which will release its reference to the IDBTransactionBackedImpl instance. If that was the last reference, the IDBTransactionBackedImpl instance is deleted and we crash.
Attachments
Make IDBTransactionBackendImpl objects take a self reference during commit and abort.
(2.07 KB, patch)
2010-12-15 09:01 PST
,
Andrei Popescu
eric
: review-
Details
Formatted Diff
Diff
test added
(5.19 KB, patch)
2010-12-17 11:16 PST
,
Andrei Popescu
jorlow
: review+
commit-queue
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Andrei Popescu
Comment 1
2010-12-15 09:01:16 PST
Created
attachment 76652
[details]
Make IDBTransactionBackendImpl objects take a self reference during commit and abort.
Eric Seidel (no email)
Comment 2
2010-12-15 15:24:51 PST
Comment on
attachment 76652
[details]
Make IDBTransactionBackendImpl objects take a self reference during commit and abort. How do existing tests cover this? Do they crash? If so, which tests are you making not crash? Please explain in your ChangeLog.
Jeremy Orlow
Comment 3
2010-12-16 02:20:25 PST
Besides Eric's comments, it looks good. In theory, it might be possible to repro this in DRT by creating a transaction, leaving the scope where it was created, doing a gc(), and then praying. It wouldn't surprise me if we couldn't come up with a reliable repro, but please give it a shot.
Andrei Popescu
Comment 4
2010-12-16 03:43:57 PST
(In reply to
comment #3
)
> Besides Eric's comments, it looks good. In theory, it might be possible to repro this in DRT by creating a transaction, leaving the scope where it was created, doing a gc(), and then praying. It wouldn't surprise me if we couldn't come up with a reliable repro, but please give it a shot.
Okay, lemme try.
Andrei Popescu
Comment 5
2010-12-17 11:16:21 PST
Created
attachment 76894
[details]
test added
Andrei Popescu
Comment 6
2010-12-17 11:18:24 PST
(In reply to
comment #3
)
> Besides Eric's comments, it looks good. In theory, it might be possible to repro this in DRT by creating a transaction, leaving the scope where it was created, doing a gc(), and then praying. It wouldn't surprise me if we couldn't come up with a reliable repro, but please give it a shot.
Ok, added a test. I can see in in Visual Studio the deletion happening before accessing member variables of the deleted instance but the test does not always crash for me as the memory isn't always reclaimed. However, if the memory is reclaimed, this test will trigger the crash.
Jeremy Orlow
Comment 7
2010-12-17 11:22:41 PST
Comment on
attachment 76894
[details]
test added r=me
Jeremy Orlow
Comment 8
2010-12-17 11:24:35 PST
(In reply to
comment #6
)
> (In reply to
comment #3
) > > Besides Eric's comments, it looks good. In theory, it might be possible to repro this in DRT by creating a transaction, leaving the scope where it was created, doing a gc(), and then praying. It wouldn't surprise me if we couldn't come up with a reliable repro, but please give it a shot. > > Ok, added a test. I can see in in Visual Studio the deletion happening before accessing member variables of the deleted instance but the test does not always crash for me as the memory isn't always reclaimed. However, if the memory is reclaimed, this test will trigger the crash.
I'm pretty sure there's nothing much more you can do here. It's definitely better than nothing and tools like Valgrind should catch the errors. Thanks.
WebKit Commit Bot
Comment 9
2010-12-20 06:00:27 PST
The commit-queue encountered the following flaky tests while processing
attachment 76894
[details]
: fast/preloader/script.html
bug 50879
(author:
abarth@webkit.org
) fast/loader/recursive-before-unload-crash.html
bug 50880
(authors:
beidson@apple.com
and
eric@webkit.org
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 10
2010-12-20 06:32:35 PST
Comment on
attachment 76894
[details]
test added Rejecting
attachment 76894
[details]
from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--non-interactive', 76894]" exit_code: 2 Last 500 characters of output: /ChangeLog Failed to merge in the changes. Patch failed at 0001 2010-12-20 Yury Semikhatsky <
yurys@chromium.org
> When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 132. Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2 Full output:
http://queues.webkit.org/results/7320063
Andrei Popescu
Comment 11
2010-12-20 06:42:46 PST
Committed
r74343
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug