title: SQLite database, multithreading, Locks and account sync on android
body: <p>I'm trying to get a pattern that doesn't fail for a multithreaded access to my sqlite database. Also, what is driving me nuts is that I can't reproduce the issue.</p>
  
  <p>I have an app which uses a DB, but also Android Accounts and Android sync to sync my app's data. My guess is that when the two happen a the same time, it crashes. I'm getting a lot of errors like:</p>
  
  <pre><code> * android.database.sqlite.SQLiteDatabaseLockedException: database is locked
   * android.database.sqlite.SQLiteDatabaseLockedException: database is locked (code 5)
   * android.database.sqlite.SQLiteDatabaseLockedException: error code 5: database is locked
   * android.database.sqlite.SQLiteDatabaseLockedException: database is locked (code 5): , while compiling: PRAGMA journal_mode
   * android.database.sqlite.SQLiteDiskIOException: disk I/O error (code 778)
   * android.database.sqlite.SQLiteException: Failed to change locale for db '/data/data/net.bicou.redmine/databases/redmine.db' to 'en_US'. \n Caused by: android.database.sqlite.SQLiteDatabaseLockedException: database is locked (code 5)
  </code></pre>
  
  <p>Maybe not all of them are related to the same root cause, however I'm kind of lost.</p>
  
  <p>What I have is:</p>
  
  <ul>
  <li>an abstract base class, <code>DbAdapter</code>, that is extended by subclasses which want to manage a single table</li>
  <li>a class that manages the SQLite database, called <code>DbManager</code>, which contains a <code>Lock</code></li>
  </ul>
  
  <p>Right now the users have a version of the <code>DbManager</code> that is not a singleton. I'm planning to make <code>DbManager</code> a singleton, so that all threads share the same object. This shouldn't be a problem, because as far as I have understood/seen, the background sync and app share the same process.</p>
  
  <p>Here are the classes (only the relevant parts):</p>
  
  <pre><code>public abstract class DbAdapter {
      Context mContext;
      protected DbManager mDbManager;
      SQLiteDatabase mDb;
  
      public static final String KEY_ROWID = "_id";
  
      public DbAdapter(final Context ctx) {
          mContext = ctx;
      }
  
      public DbAdapter(final DbAdapter other) {
          mContext = other.mContext;
          mDb = other.mDb;
          mDbManager = other.mDbManager; // removed with singleton version
      }
  
      public synchronized DbAdapter open() throws SQLException {
          if (mDb != null) {
              return this;
          }
  
          mDbManager = new DbManager(mContext); // currently in production
          mDbManager = DbManager.instance(mContext); // currently investigating this singleton solution
          try {
              mDb = mDbManager.getWritableDatabase();
          } catch (final SQLException e) {
              L.e("Unable to open DB, trying again in 1 second", e);
              try {
                  Thread.sleep(1000);
              } catch (final InterruptedException e1) {
                  L.e("Could not wait 1 second " + e1);
              }
              mDb = mDbManager.getWritableDatabase();// This may crash
          }
  
          return this;
      }
  
      public synchronized void close() {
          mDbManager.close();
          mDbManager = null;
          mDb = null;
      }
  }
  </code></pre>
  
  <p>A class that needs to handle a database table will extend <code>DbAdapter</code>, and implement methods such as <code>select</code>, <code>insert</code>, <code>delete</code>, etc.</p>
  
  <p>Here's the DB manager:</p>
  
  <pre><code>public class DbManager extends SQLiteOpenHelper {
      private static final String DB_FILE = "db";
      private static final int DB_VERSION = 15;
      Context mContext;
      Lock mLock = new ReentrantLock();
  
      // Currently in prod
      public DbManager(final Context context) {
          super(context, DB_FILE, null, DB_VERSION);
          mContext = context;
      }
  
      // singleton version will make this constructor private and add:
      private static DbManager mInstance;
      public static synchronized DbManager instance(Context context) {
          if (instance == null) {
              instance = new DbManager(context);
          }
          return instance;
      }
  
      @Override
      public SQLiteDatabase getWritableDatabase() {
          mLock.lock();
          return super.getWritableDatabase();
      }
  
      @Override
      public void close() {
          super.close();
          mLock.unlock();
      }
  
      @Override
      public void onCreate(final SQLiteDatabase db) {
          // ...
      }
  
      @Override
      public void onUpgrade(final SQLiteDatabase db, final int oldVersion, final int newVersion) {
          // ...
      }
  
      private void createTables(final SQLiteDatabase db, final String[] statements) {
          for (final String sql : statements) {
              try {
                  db.execSQL(sql);
              } catch (final Exception e) {
                  L.e("Unable to create table: " + sql, e);
              }
          }
      }
  }
  </code></pre>
  
  <hr>
  
  <p>OK, now, the questions.</p>
  
  <ol>
  <li>Is my lock properly implemented? I'm really new to this, I don't know if the <code>ReentrantLock</code> is a good choice, and if I'm locking/unlocking at the right moment</li>
  <li>Are my <code>synchronized</code> method properly implemented? I mean, I have placed the <code>synchronized</code> keyword around methods that I don't want interrupted by concurrent threads. Is this right? Can you advice on my <code>synchronized</code> use?</li>
  <li>How can I reproduce the issue? I have created a test that uses 3 threads that make concurrent read/write access to the DB, and use some <code>Thread.sleep</code> to ensure that the db open/close from each thread overlap, but it doesn't crash. This is really bugging me, I don't think there is a lot of people that have the issue, so I don't know how to reproduce.</li>
  <li>Is my <code>DbAdapter</code> + <code>DbManager</code> technical choice a good idea? Is there a better pattern?</li>
  <li>Is it a good idea to make <code>DbManager</code> a singleton?</li>
  </ol>