[Webkit-unassigned] [Bug 25711] HTML5 Database becomes locked if a transaction is in progress when the page is refreshed.

[bugzilla-daemon at webkit.org]

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





------- Comment #36 from koivisto at iki.fi  2009-07-02 15:04 PDT -------
(In reply to comment #35)
> Hi Antti!
> 
> Thanks for the review, glad that we're very almost done!
> 
> > Is it really always true that recordDatabaseOpen/recordDatabaseClose can't get
> > called in the database thread? If so then obviously you won't need mutexes, but
> > please make sure.
> 
> Yes, at the moment the only times recoredDatabaseOpen/Close are called are in
> DatabaseOpen/Close tasks that run on the database thread, or in the code I
> added to close the databases before the thread terminates (naturally on the
> database thread).
> 
> > > +    // Close the databases that we ran transactions on. This ensures that if any transactions are still open, they are rolled back and we don't leave the database in an
> > > +    // inconsistent or locked state.
> > > +    if (m_openDatabaseSet.size() > 0) {
> > > +        // As the call to close will modify the original set, we must take a copy to iterate over.
> > > +        DatabaseSet openSetCopy = m_openDatabaseSet;
> > > +        DatabaseSet::iterator end = openSetCopy.end();
> > > +        for (DatabaseSet::iterator it = openSetCopy.begin(); it != end; ++it)
> > > +           (*it)->close();
> > 
> > You should use HashSet::swap() instead of making a copy before iterating.
> 
> OK, but if I do that then in the case where a database is closed by this code,
> the ASSERT(m_openDatabaseSet.contains(database)) in recordDatabaseClosed()
> called by Database::close() will fail as we will have swapped the set with an
> empty set. What is preferable? Keep as is and do the copy and ASSERT in
> recordDatabaseClosed (I don't think this is often going to be a very large
> copy) or use swap instead of copy and remove the assert? In my opinion keeping
> the assert is nicer. What do you think?

I think you could just change the assert: ASSERT(m_queue.killed() ||
m_openDatabaseSet.contains(database)).

Asserts as they are won't catch anything interesting during termination (you
obviously are removing only databases that are in the set since you are
iterating the set). 


> Thanks, Ben
> 


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