From 2df6b358feddb2fc257c15ed4826fc6e322292bd Mon Sep 17 00:00:00 2001
From: Jason Madden <jamadden@gmail.com>
Date: Sat, 9 Jul 2016 10:44:01 -0500
Subject: [PATCH] Tweak locks for new_oid.

---
 relstorage/storage.py           | 10 +++++++++-
 relstorage/tests/reltestbase.py | 28 +++-------------------------
 2 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/relstorage/storage.py b/relstorage/storage.py
index ac8c3a4d..6e989a3c 100644
--- a/relstorage/storage.py
+++ b/relstorage/storage.py
@@ -179,7 +179,10 @@ class RelStorage(UndoLogCompatible,
 
     def __init__(self, adapter, name=None, create=None,
                  options=None, cache=None, blobhelper=None,
-                 _use_locks=False,
+                 # The top-level storage should use locks because
+                 # new_oid is shared among all connections. But the new_instance
+                 # objects don't need to.
+                 _use_locks=True,
                  **kwoptions):
         self._adapter = adapter
 
@@ -269,6 +272,7 @@ def new_instance(self):
             blobhelper = None
         other = type(self)(adapter=adapter, name=self.__name__,
                            create=False, options=self._options, cache=cache,
+                           _use_locks=False,
                            blobhelper=blobhelper)
         self._instances.append(weakref.ref(other, self._instances.remove))
 
@@ -1062,6 +1066,10 @@ def new_oid(self):
         if self._is_read_only:
             raise ReadOnlyError()
         with self._lock:
+            # This method is actually called on the storage object of
+            # the DB, not the storage object of a connection for some
+            # reason. This means this method is shared among all
+            # connections using a database.
             if self._preallocated_oids:
                 oid_int = self._preallocated_oids.pop()
             else:
diff --git a/relstorage/tests/reltestbase.py b/relstorage/tests/reltestbase.py
index 705ab0df..4a1af98b 100644
--- a/relstorage/tests/reltestbase.py
+++ b/relstorage/tests/reltestbase.py
@@ -41,7 +41,6 @@
 class StorageCreatingMixin(object):
 
     keep_history = None # Override
-    use_locked_storage = None # override
 
     def make_adapter(self, options):
         # abstract method
@@ -60,7 +59,7 @@ def make_storage(self, zap=True, **kw):
 
         options = Options(keep_history=self.keep_history, **kw)
         adapter = self.make_adapter(options)
-        storage = RelStorage(adapter, options=options, _use_locks=self.use_locked_storage)
+        storage = RelStorage(adapter, options=options)
         storage._batcher_row_limit = 1
         if zap:
             # XXX: Some ZODB tests, possibly check4ExtStorageThread and
@@ -76,38 +75,17 @@ class RelStorageTestBase(StorageCreatingMixin,
 
     keep_history = None  # Override
     _storage_created = None
-    _locked_tests = (
-        # These tests know nothing of IMVCCStorage and so don't
-        # create a new_instance for each thread, as of ZODB 4.3.1.
-        # BasicStorage
-        'check_checkCurrentSerialInTransaction',
-        'check_tid_ordering_w_commit',
-        # MTStorage
-        'check2StorageThreads',
-        'check7StorageThreads',
-        'check4ExtStorageThread',
-        # XXX These two MTStorage tests don't actually need locks. But
-        # on TravisCI, both Python 2.7 and 3.4 segfaulted (sometimes!) in one of
-        # these tests when run under Coverage, so we include them anyway.
-        'check2ZODBThreads',
-        'check7ZODBThreads',
-        # PackableStorage
-        'checkPackWhileWriting',
-        'checkPackNowWhileWriting',
-        'checkPackLotsWhileWriting',
-    )
 
     def setUp(self):
-        if self._testMethodName in self._locked_tests:
-            self.use_locked_storage = True
+        pass
         # Note that we're deliberately NOT calling super's setup.
         # It does stuff on disk, etc, that's not necessary for us
         # and just slows us down by ~10%.
         #super(RelStorageTestBase, self).setUp()
 
     def tearDown(self):
-        self.use_locked_storage = False
         transaction.abort()
+        # XXX: This could create one! Do subclasses override self._storage?
         storage = self._storage
         if storage is not None:
             self._storage = None