[refactor] remove threadpool from sqlcipher backend

The old strategy for having Soledad sync running asynchronously with other
API calls was to have the sync running in its own threadpool. This is not
needed now that all sync code uses deferreds and will not block. This commit
removes what's left from the old threadpool.

2 files changed, 18 insertions, 54 deletions

diff --git a/client/src/leap/soledad/client/sqlcipher.py b/client/src/leap/soledad/client/sqlcipher.py
index 18a9c5de..2151884a 100644
--- a/client/src/leap/soledad/client/sqlcipher.py
+++ b/client/src/leap/soledad/client/sqlcipher.py
@@ -55,8 +55,7 @@ from functools import partial
 from pysqlcipher import dbapi2 as sqlcipher_dbapi2
 
 from twisted.internet import reactor
-from twisted.internet.threads import deferToThreadPool
-from twisted.python.threadpool import ThreadPool
+from twisted.internet import defer
 from twisted.enterprise import adbapi
 
 from leap.soledad.client.http_target import SoledadHTTPSyncTarget
@@ -445,8 +444,6 @@ class SQLCipherU1DBSync(SQLCipherDatabase):
         self.received_docs = []
 
         self.running = False
-        self._sync_threadpool = None
-        self._initialize_sync_threadpool()
 
         self._reactor = reactor
         self._reactor.callWhenRunning(self._start)
@@ -462,54 +459,27 @@ class SQLCipherU1DBSync(SQLCipherDatabase):
 
     def _start(self):
         if not self.running:
-            self._sync_threadpool.start()
             self.shutdownID = self._reactor.addSystemEventTrigger(
                 'during', 'shutdown', self.finalClose)
             self.running = True
 
-    def _defer_to_sync_threadpool(self, meth, *args, **kwargs):
-        return deferToThreadPool(
-            self._reactor, self._sync_threadpool, meth, *args, **kwargs)
-
     def _initialize_main_db(self):
+        self._db_handle = initialize_sqlcipher_db(
+            self._opts, check_same_thread=False)
+        self._real_replica_uid = None
+        self._ensure_schema()
+        self.set_document_factory(soledad_doc_factory)
 
-        def _init_db():
-            self._db_handle = initialize_sqlcipher_db(
-                self._opts, check_same_thread=False)
-            self._real_replica_uid = None
-            self._ensure_schema()
-            self.set_document_factory(soledad_doc_factory)
-
-        return self._defer_to_sync_threadpool(_init_db)
-
-    def _initialize_sync_threadpool(self):
-        """
-        Initialize a ThreadPool with exactly one thread, that will be used to
-        run all the network blocking calls for syncing on a separate thread.
-
-        TODO this needs to be ported away from urllib and into twisted async
-        calls, and then we can ditch this syncing thread and reintegrate into
-        the main reactor.
-        """
-        # XXX if the number of threads in this thread pool is ever changed, we
-        #     should make sure that no operations on the database shuold occur
-        #     before the database has been initialized.
-        self._sync_threadpool = ThreadPool(0, 1)
-
+    @defer.inlineCallbacks
     def sync(self, url, creds=None, defer_decryption=True):
         """
         Synchronize documents with remote replica exposed at url.
 
-        This method defers a sync to a 1-threaded threadpool. The main
-        database initialziation was deferred to that thread during this
-        object's initialization. As there's currently only one thread in that
-        threadpool, the db init was queued before this method was called, so
-        we don't need to actually wait for the db to be ready. If this ever
-        changes, we should add a thread-safe condition to ensure the db is
-        ready before using it.
-
-        This method is atomic, with atomicity guaranteed by caller. Each
-        caller should implement a locking mechanism to ensure atomicity.
+        It is not safe to initiate more than one sync process and let them run
+        concurrently. It is responsibility of the caller to ensure that there
+        are no concurrent sync processes running. This is currently controlled
+        by the main Soledad object because it may also run post-sync hooks,
+        which should be run while the lock is locked.
 
         :param url: The url of the target replica to sync with.
         :type url: str
@@ -527,16 +497,10 @@ class SQLCipherU1DBSync(SQLCipherDatabase):
         :rtype: Deferred
         """
         syncer = self._get_syncer(url, creds=creds)
-
-        def _record_received_docs(result):
-            # beware, closure. syncer is in scope.
-            self.received_docs = syncer.received_docs
-            return result
-
-        # XXX could mark the critical section here...
-        d = syncer.sync(defer_decryption=defer_decryption)
-        d.addCallback(_record_received_docs)
-        return d
+        local_gen_before_sync = yield syncer.sync(
+            defer_decryption=defer_decryption)
+        self.received_docs = syncer.received_docs
+        defer.returnValue(local_gen_before_sync)
 
     def _get_syncer(self, url, creds=None):
         """
@@ -589,7 +553,6 @@ class SQLCipherU1DBSync(SQLCipherDatabase):
         This should only be called by the shutdown trigger.
         """
         self.shutdownID = None
-        self._sync_threadpool.stop()
         self.running = False
 
     def close(self):
diff --git a/client/src/leap/soledad/client/sync.py b/client/src/leap/soledad/client/sync.py
index deb183ca..110baa0a 100644
--- a/client/src/leap/soledad/client/sync.py
+++ b/client/src/leap/soledad/client/sync.py
@@ -59,7 +59,8 @@ class SoledadSynchronizer(Synchronizer):
                                  decryption will be done inline.
         :type defer_decryption: bool
 
-        :return: A deferred which will fire after the sync has finished.
+        :return: A deferred which will fire after the sync has finished with
+                 the local generation before the synchronization was performed.
         :rtype: twisted.internet.defer.Deferred
         """
         sync_target = self.sync_target