Remove DBWRAPPER for sqlcipher access.

We're using instead the very useful but undocumented
check_same_thread parameter in dbapi2.connect, plus
class level locks when accessing a sqlcipher database.

3 files changed, 49 insertions, 256 deletions

diff --git a/soledad/src/leap/soledad/__init__.py b/soledad/src/leap/soledad/__init__.py
index 94425d66..fc0c10ce 100644
--- a/soledad/src/leap/soledad/__init__.py
+++ b/soledad/src/leap/soledad/__init__.py
@@ -125,11 +125,10 @@ except ImportError:
 
 
 from leap.soledad.crypto import SoledadCrypto
-from leap.soledad.dbwrapper import SQLCipherWrapper
 from leap.soledad.document import SoledadDocument
 from leap.soledad.shared_db import SoledadSharedDatabase
-#from leap.soledad.sqlcipher import open as sqlcipher_open
-#from leap.soledad.sqlcipher import SQLCipherDatabase
+from leap.soledad.sqlcipher import open as sqlcipher_open
+from leap.soledad.sqlcipher import SQLCipherDatabase
 from leap.soledad.target import SoledadSyncTarget
 
 
@@ -429,8 +428,7 @@ class Soledad(object):
             buflen=32,  # we need a key with 256 bits (32 bytes)
         )
 
-        # Instantiate a thread-safe wrapper
-        self._db = SQLCipherWrapper(
+        self._db = sqlcipher_open(
             self._local_db_path,
             binascii.b2a_hex(key),  # sqlcipher only accepts the hex version
             create=True,
@@ -444,7 +442,7 @@ class Soledad(object):
         """
         if hasattr(self, '_db') and isinstance(
                 self._db,
-                SQLCipherWrapper):
+                SQLCipherDatabase):
             self._db.close()
 
     def __del__(self):
diff --git a/soledad/src/leap/soledad/dbwrapper.py b/soledad/src/leap/soledad/dbwrapper.py
deleted file mode 100644
index 7e134d1b..00000000
--- a/soledad/src/leap/soledad/dbwrapper.py
+++ /dev/null
@@ -1,218 +0,0 @@
-# -*- coding: utf-8 -*-
-# dbwrapper.py
-# Copyright (C) 2013 LEAP
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-"""
-Thread-safe wrapper for sqlite/pysqlcipher.
-
-*TODO*
-At some point we surely will want to switch to a twisted way of dealing
-with this, using defers and proper callbacks. But I had this tested for
-some time so postponing that refactor.
-"""
-import logging
-import threading
-import Queue
-
-import exceptions
-
-from functools import partial
-
-from leap.soledad import sqlcipher
-
-logger = logging.getLogger(__name__)
-
-
-class SQLCipherWrapper(threading.Thread):
-
-    k_lock = threading.Lock()
-
-    def __init__(self, *args, **kwargs):
-        """
-        Initializes a wrapper that proxies method and attribute
-        access to an underlying SQLCipher instance. We instantiate sqlcipher
-        in a thread, and all method accesses communicate with it using a
-        Queue.
-
-        :param *args: position arguments to pass to pysqlcipher initialization
-        :type args: tuple
-
-        :param **kwargs: keyword arguments to pass to pysqlcipher
-                         initialization
-        :type kwargs: dict
-        """
-        threading.Thread.__init__(self)
-        self._lock = threading.Lock()
-        self._db = None
-        self._wrargs = args, kwargs
-
-        self._queue = Queue.Queue()
-        self._stopped = threading.Event()
-
-        self.start()
-
-    def _init_db(self):
-        """
-        Initializes sqlcipher database.
-
-        This is called on a separate thread.
-        """
-        # instantiate u1db
-        args, kwargs = self._wrargs
-        with self.k_lock:
-            try:
-                self._db = sqlcipher.open(*args, **kwargs)
-                print "init ok"
-            except Exception as exc:
-                logger.debug("Error in init_db: %r" % (exc,))
-                self._stopped.set()
-                raise exc
-
-    def run(self):
-        """
-        Main loop for the sqlcipher thread.
-        """
-        END_METHODS = ("__end_thread", "_SQLCipherWrapper__end_thread")
-        logger.debug("SQLCipherWrapper thread started.")
-        logger.debug("Initializing sqlcipher")
-
-        #ct = 0
-        #started = False
-
-        try:
-            self._init_db()
-        except:
-            logger.error("Failed to init db.")
-            print "error on init"
-            # XXX should raise?
-
-        while True:
-
-            #if self._db is None:
-                #if started:
-                    #break
-                #if ct > 10:
-                    #break  # XXX DEBUG
-                #logger.debug('db not ready yet, waiting...')
-                #print "db not ready, wait..."
-                #time.sleep(1)
-                #ct += 1
-
-            if self._db is None:
-                print "db not initialized!"
-
-            #started = True
-            with self._lock:
-
-                try:
-                    # XXX not getting args...
-                    mth, q, wrargs = self._queue.get()  # False?
-                except:
-                    print "Exception getting args"
-                    logger.error("exception getting args from queue")
-                    continue
-
-                print "mth: ", mth
-                res = None
-                attr = getattr(self._db, mth, None)
-                if not attr:
-                    if mth not in END_METHODS:
-                        logger.error('method %s does not exist' % (mth,))
-                        res = AttributeError(
-                            "_db instance has no attribute %s" % mth)
-
-                elif callable(attr):
-                    # invoke the method with the passed args
-                    args = wrargs.get('args', [])
-                    kwargs = wrargs.get('kwargs', {})
-                    try:
-                        res = attr(*args, **kwargs)
-                    except Exception as e:
-                        logger.error(
-                            "Error on proxied method %s: '%r'." % (
-                            attr, e))
-                        res = e
-                else:
-                    # non-callable attribute
-                    res = attr
-                #logger.debug('returning proxied db call...')
-                print "putting res: ", res
-                q.put(res)
-
-                if mth in END_METHODS:
-                    logger.debug('ending thread')
-                    break
-
-        logger.debug("SQLCipherWrapper thread terminated.")
-        print "SQLWRAPPER TERMINATED"
-        self._stopped.set()
-
-    def close(self):
-        """
-        Closes the sqlcipher database and finishes the thread. This method
-        should always be called explicitely.
-        """
-        self.__getattr__('close')()
-        self.__end_thread()
-
-    def __getattr__(self, attr):
-        """
-        Returns _db proxied attributes.
-        """
-
-        def __proxied_mth(method, *args, **kwargs):
-            SQLWRAPPER_METHOD_TIMEOUT = 20  # in seconds
-            if not self._stopped.isSet():
-                wrargs = {'args': args, 'kwargs': kwargs}
-                q = Queue.Queue()
-                print "put meth"
-                with self.k_lock:
-                    self._queue.put((method, q, wrargs))
-                    try:
-                        # XXX this is blocking
-                        res = q.get(timeout=SQLWRAPPER_METHOD_TIMEOUT)
-                        q.task_done()
-                    except Queue.Empty:
-                        res = None
-                    print "got result"
-
-                if isinstance(res, exceptions.BaseException):
-                    # XXX should get the original bt
-                    raise res
-                return res
-            else:
-                print "called but stopped!"
-                logger.warning("tried to call proxied meth "
-                               "but stopped is set: %s" %
-                               (method,))
-
-        rgetattr = object.__getattribute__
-
-        if attr != "_db":
-            proxied = partial(__proxied_mth, attr)
-            return proxied
-
-        # fallback to regular behavior
-        return rgetattr(self, attr)
-
-    def __del__(self):
-        """
-        Closes the thread upon object destruction.
-
-        Do not trust this get called. No guarantees given. Because of a funny
-        dance with the refs and the way the gc works, we should be calling the
-        close method explicitely.
-        """
-        self.close()
diff --git a/soledad/src/leap/soledad/sqlcipher.py b/soledad/src/leap/soledad/sqlcipher.py
index 18a8fe6e..94d0fb04 100644
--- a/soledad/src/leap/soledad/sqlcipher.py
+++ b/soledad/src/leap/soledad/sqlcipher.py
@@ -61,6 +61,15 @@ logger = logging.getLogger(__name__)
 # Monkey-patch u1db.backends.sqlite_backend with pysqlcipher.dbapi2
 sqlite_backend.dbapi2 = dbapi2
 
+# It seems that, as long as we are not using old sqlite versions, serialized
+# mode is enabled by default at compile time. So accessing db connections from
+# different threads should be safe, as long as no attempt is made to use them
+# from multiple threads with no locking.
+# See https://sqlite.org/threadsafe.html
+# and http://bugs.python.org/issue16509
+
+SQLITE_CHECK_SAME_THREAD = False
+
 
 def open(path, password, create=True, document_factory=None, crypto=None,
          raw_key=False, cipher='aes-256-cbc', kdf_iter=4000,
@@ -161,7 +170,9 @@ class SQLCipherDatabase(sqlite_backend.SQLitePartialExpandDatabase):
                 cipher_page_size)
         # connect to the database
         with self.k_lock:
-            self._db_handle = dbapi2.connect(sqlcipher_file)
+            self._db_handle = dbapi2.connect(
+                sqlcipher_file,
+                check_same_thread=SQLITE_CHECK_SAME_THREAD)
             # set SQLCipher cryptographic parameters
             self._set_crypto_pragmas(
                 self._db_handle, password, raw_key, cipher, kdf_iter,
@@ -211,40 +222,37 @@ class SQLCipherDatabase(sqlite_backend.SQLitePartialExpandDatabase):
             raise u1db_errors.DatabaseDoesNotExist()
 
         tries = 2
+        # Note: There seems to be a bug in sqlite 3.5.9 (with python2.6)
+        #       where without re-opening the database on Windows, it
+        #       doesn't see the transaction that was just committed
         while True:
-            print "tries: ", tries
-            # Note: There seems to be a bug in sqlite 3.5.9 (with python2.6)
-            #       where without re-opening the database on Windows, it
-            #       doesn't see the transaction that was just committed
-
-            db_handle = dbapi2.connect(sqlcipher_file)
 
-            try:
-                # set cryptographic params
-                cls._set_crypto_pragmas(
-                    db_handle, password, raw_key, cipher, kdf_iter,
-                    cipher_page_size)
-                c = db_handle.cursor()
-                # XXX if we use it here, it should be public
-                v, err = cls._which_index_storage(c)
-            except Exception as exc:
-                logger.warning("ERROR OPENING DATABASE!")
-                print "ERROR OPENING DB ---------"
-                logger.debug("error was: %r" % exc)
-                print "error was: %r" % exc
-                print exc.__class__
-                v, err = None, exc
-            finally:
-                db_handle.close()
-            if v is not None:
-                break
+            with cls.k_lock:
+                db_handle = dbapi2.connect(
+                    sqlcipher_file,
+                    check_same_thread=SQLITE_CHECK_SAME_THREAD)
+
+                try:
+                    # set cryptographic params
+                    cls._set_crypto_pragmas(
+                        db_handle, password, raw_key, cipher, kdf_iter,
+                        cipher_page_size)
+                    c = db_handle.cursor()
+                    # XXX if we use it here, it should be public
+                    v, err = cls._which_index_storage(c)
+                except Exception as exc:
+                    logger.warning("ERROR OPENING DATABASE!")
+                    logger.debug("error was: %r" % exc)
+                    v, err = None, exc
+                finally:
+                    db_handle.close()
+                if v is not None:
+                    break
             # possibly another process is initializing it, wait for it to be
             # done
             if tries == 0:
-                print "ERROR, raise"
                 raise err  # go for the richest error?
             tries -= 1
-            print "sleeping"
             time.sleep(cls.WAIT_FOR_PARALLEL_INIT_HALF_INTERVAL)
         return SQLCipherDatabase._sqlite_registry[v](
             sqlcipher_file, password, document_factory=document_factory,
@@ -422,10 +430,15 @@ class SQLCipherDatabase(sqlite_backend.SQLitePartialExpandDatabase):
         except dbapi2.DatabaseError:
             # assert that we can access it using SQLCipher with the given
             # key
-            db_handle = dbapi2.connect(sqlcipher_file)
-            cls._set_crypto_pragmas(
-                db_handle, key, raw_key, cipher, kdf_iter, cipher_page_size)
-            db_handle.cursor().execute('SELECT count(*) FROM sqlite_master')
+            with cls.k_lock:
+                db_handle = dbapi2.connect(
+                    sqlcipher_file,
+                    check_same_thread=SQLITE_CHECK_SAME_THREAD)
+                cls._set_crypto_pragmas(
+                    db_handle, key, raw_key, cipher,
+                    kdf_iter, cipher_page_size)
+                db_handle.cursor().execute(
+                    'SELECT count(*) FROM sqlite_master')
 
     @classmethod
     def _set_crypto_pragmas(cls, db_handle, key, raw_key, cipher, kdf_iter,