[Webkit-unassigned] [Bug 27966] Race condition in the database code

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 7 14:16:10 PDT 2009


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
  Attachment #34299|review?                     |review-
               Flag|                            |

--- Comment #10 from Eric Seidel <eric at webkit.org>  2009-08-07 14:16:07 PDT ---
(From update of attachment 34299)
Thank you!  This is much much more reviewable.

Why do we have one transaction coordinator per thread instead of per database? 
if it was per database, then we would not need the hash to hold these queues,
we could just hold them on each datbase?  Do transations span across datbases?

I find the test case very hard to read using anonymous functions.  If I were
writing that I would have given the functions nice names, even if they were
only defined locally.

It seems teh select/insert tests differ only in "db1" vs. "db2".  Probably
should be a single funtion which takes a name argument, no?

Likewise the openDatbasse call could be a function to make this more readable. 
In general the test has too much copy/paste code. :(

Since no one else has stepped up yet, I think you're stuck with me! ;)

In general this looks great.  The layout test needs some cleanup.  And I need
better understanding (ideally with explanation in teh ChagneLog) as to why it's
designed this way with teh DatbaseThread holding the transaction coordinator
instead of the datbase.  Also confusing that m_database is actually a
datbaseThread it seems.

Why is a securityORiginCopy needeD?
 62     return database->securityOriginCopy()->databaseIdentifier() + ":" +

In general the ChangeLog could be more detailed.

anyway, I look forward to the next round!  This seems like a good fix.  Just
needs to be presented more clearly.

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list