Bug 27947 - A bug in SQLiteTransaction can lead to a failed ASSERT
Summary: A bug in SQLiteTransaction can lead to a failed ASSERT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Dumitru Daniliuc
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-03 11:36 PDT by Dumitru Daniliuc
Modified: 2009-08-03 16:00 PDT (History)
5 users (show)

See Also:


Attachments
patch (1.95 KB, patch)
2009-08-03 12:28 PDT, Dumitru Daniliuc
darin: review+
Details | Formatted Diff | Diff
test case (1.56 KB, text/html)
2009-08-03 13:44 PDT, Dumitru Daniliuc
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dumitru Daniliuc 2009-08-03 11:36:30 PDT
SQLiteTransaction::begin() sets m_db.m_transactionInProgress to true even if m_db.executeCommand("BEGIN;") fails. This can lead to an ASSERT failing incorrectly in SQLTransaction::openTransaction() (steps 1 + 2).
Comment 1 Dumitru Daniliuc 2009-08-03 12:28:17 PDT
Created attachment 33997 [details]
patch
Comment 2 Darin Adler 2009-08-03 12:46:27 PDT
Comment on attachment 33997 [details]
patch

Seems fine. Is there any way to make a regression test for this? Under what circumstances will the BEGIN command fail?

r=me
Comment 3 Dimitri Glazkov (Google) 2009-08-03 12:56:11 PDT
Should we need another assert after BEGIN. This shouldn't be happening normally, right?

It seems like we should be doing something more than just reporting !inProgress in these cases.
Comment 4 Dumitru Daniliuc 2009-08-03 13:44:22 PDT
Created attachment 34000 [details]
test case

(In reply to comment #2)
> (From update of attachment 33997 [details])
> Seems fine. Is there any way to make a regression test for this? Under what
> circumstances will the BEGIN command fail?
> 
> r=me

BEGIN seems to fail if the same DB thread tries to run 2 transactions in 2 different Database objects that point to the same DB file. I attached the example we used.

I don't have a detailed explanation for what's going on yet -- we think there's a race condition here and I'm still debugging it (details + patch to come in a future bug). However, always setting m_db.m_transactionInProgress to true seems wrong and leads to crashes in Chromium's and Safari's debug builds (you might have to click the "Test" button a couple of times). So I thought it wouldn't hurt to submit a quick patch for this before we're done investigating the bigger issue.
Comment 5 Dimitri Glazkov (Google) 2009-08-03 16:00:11 PDT
Landed as http://trac.webkit.org/changeset/46736.

Please create a bug for tracking the origin of the issue.