[webkit-reviews] review canceled: [Bug 27966] Race condition in the database code : [Attachment 34363] patch + test case

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 10 14:13:43 PDT 2009


Dumitru Daniliuc <dumi at chromium.org> has canceled Dumitru Daniliuc
<dumi at chromium.org>'s request for review:
Bug 27966: Race condition in the database code
https://bugs.webkit.org/show_bug.cgi?id=27966

Attachment 34363: patch + test case
https://bugs.webkit.org/attachment.cgi?id=34363&action=review

------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
Michael, I think I addressed all your comments, please take another look.

m_lockAcquired: I added ASSERTs at the beginning of every task executed on the
DB thread (+ openTransactionAndPreflight, which used to be and still kind of is
the real first step). I'm also checking this flag in the destructor, and if
it's true, I call releaseLock(). Any reason not to do it? It seems to me like
this approach should work in Chromium's case too. Minor concern: in order to be
able to do this in the destructor, I have to set m_lockAcquired to false
everywhere I call releaseLock() (otherwise releaseLock() will be called in the
destructor again, and some assertions will fail). Is it somehow possible that
releaseLock() is called (and the transaction is taken out of the queue), but
m_lockAcquired is not reset to false right after that? I can't think of any
case that would result in this behavior.

HashMap of collections: I used the same technique that you used in your code,
and cleaned up the destructor. Please double-check that I don't have
"refactoring typos" in this code.


More information about the webkit-reviews mailing list