| Summary: | IDBTransactionBackedImpl instances can be accidentally deleted during calls to abort/commit. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andrei Popescu <andreip> | ||||||
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, hans, jorlow | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Andrei Popescu
2010-12-15 08:45:12 PST
Created attachment 76652 [details]
Make IDBTransactionBackendImpl objects take a self reference during commit and abort.
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.
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. (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. Created attachment 76894 [details]
test added
(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. Comment on attachment 76894 [details]
test added
r=me
(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. 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. 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 Committed r74343 |