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

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27966


Dumitru Daniliuc <dumi at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34233|review?                     |
               Flag|                            |
  Attachment #34233|0                           |1
        is obsolete|                            |
  Attachment #34241|                            |review?
               Flag|                            |




--- Comment #5 from Dumitru Daniliuc <dumi at chromium.org>  2009-08-06 18:19:02 PDT ---
Created an attachment (id=34241)
 --> (https://bugs.webkit.org/attachment.cgi?id=34241)
patch + test case

(In reply to comment #4)

good comments!

> 50         OwnPtr<HashMap<String, Deque<SQLTransaction*>*> >
> m_pendingTransactions;
> 
> I think you could put RefPtr<T>'s in the Deque to eliminate the manual
> refcounting.

done. any way to do the same for the Deques? that is, can we wrap them into
something that would delete them as soon as we exit the block of code that
removes them from the hash map?

> 51         Mutex m_pendingTransactionsGuard;
> 
> I think this should only be called on a particular DB thread, is the Mutex
> really needed?

good catch, removed.

> 47 SQLTransactionCoordinator::~SQLTransactionCoordinator()
> 48 {
> 49 }
> 
> What happens to transactions that don't actually start prior to the thread
> being killed. How do they get derefed?

added code to clean up all pending transactions. i left it empty mostly because
i wasn't sure what to do here and was hoping for some comments. is there
anything else we might need to do other than cleaning up the transaction
queues?

> 210 void SQLTransaction::lockAquired()
> 211 {
> 212     m_nextStep =
> &SQLTransaction::deliverOpenTransactionAndPreflightCallback;
> 213     LOG(StorageAPI, "Scheduling deliverOpenTransactionAndPreflight for
> transaction %p\n", this);
> 214     m_database->scheduleTransactionCallback(this);
> 215 }
> 
> A the lockAcquired point, the system pings-pongs to the main thread and back
> again to the db thread. What is the reason for the
> 'deliverOpenTransactionAndPreflightCallback' step? I'm wondering if we can we
> skip straight to the openTransactionAndPreflight step at this point, without
> ping/ponging to the main thread.

changed lockAquired() to call openTransactionAndPreflight() directly. i
scheduled a task on the main thread only because i wanted to release the lock
on SQLTransactionCoordinator::m_pendingTransactions as soon as possible (even
though i guess it didn't matter, because the DB thread was doing the same
amount of work in one order or another). but now that the lock is gone i don't
think there's any reason to do this anymore.

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