xbmc/xbmc — pull request #664 by jmarshallnz (2012-01-27 02:15:12+00:00)
Title: Guard db upgrades

I'm not sure if this is required (see http://trac.xbmc.org/ticket/12157 for example) but it seems there's some potential for nastiness if 2 or more threads call in to do a db update.

Several things might happen:

1. Threads could compete to do the database copy (on update we copy the old db to a new one).
2. Thread B might open the "new" db while thread A is still copying it (not sure if sqlite/mysql is threadsafe in this respect - my guess is they are).
3. Thread B might copy after thread A has already copied the db, basically redoing anything that thread A has done.
4. Two threads might call into UpdateOldVersion at the same time. They can't process at the same time as it's done in a transaction, however once thread A has finished updating (assuming successful) thread B would then run through the same code and fail as we've already updated.

I don't like the way it's been done as the lock is held in situations it doesn't need to be (eg when no update is needed at all) - a nicer solution might be to have CApplication do the db update (i.e. move the updating of db's out of CDatabase::Open), and then "just" check that we're not updating (guarded) in Open(). This keeps the guard held for a minimal length of time (effectively a boolean read).