[Zodb-checkins] SVN: ZODB/trunk/src/ZEO/ Fixed a bug in the iterator support. Iterators weren't properly

[Jim Fulton]

Log message for revision 93014:
  Fixed a bug in the iterator support.  Iterators weren't properly
  cleaned up on disconnect.
  
  Also avoid unnecessary iterator_gc calls to the server when there are
  no iterators to be cleaned up.
  

Changed:
  U   ZODB/trunk/src/ZEO/ClientStorage.py
  U   ZODB/trunk/src/ZEO/tests/IterationTests.py
  U   ZODB/trunk/src/ZEO/tests/testZEO.py

-=-
Modified: ZODB/trunk/src/ZEO/ClientStorage.py
===================================================================
--- ZODB/trunk/src/ZEO/ClientStorage.py	2008-11-16 17:11:43 UTC (rev 93013)
+++ ZODB/trunk/src/ZEO/ClientStorage.py	2008-11-16 17:11:45 UTC (rev 93014)
@@ -624,7 +624,7 @@
         self._ready.clear()
         self._server = disconnected_stub
         self._midtxn_disconnect = 1
-        self._iterator_gc()
+        self._iterator_gc(True)
 
     def __len__(self):
         """Return the size of the storage."""
@@ -1380,19 +1380,26 @@
         self._iterators.pop(iid, None)
         self._iterator_ids.remove(iid)
 
-    def _iterator_gc(self):
+    def _iterator_gc(self, disconnected=False):
+        if not self._iterator_ids:
+            return
+
+        if disconnected:
+            for i in self._iterators.values():
+                i._iid = -1
+            self._iterators.clear()
+            self._iterator_ids.clear()
+            return
+
         iids = self._iterator_ids - set(self._iterators)
-        try:
-            self._server.iterator_gc(list(iids))
-        except ClientDisconnected:
-            # We could not successfully garbage-collect iterators.
-            # The server might have been restarted, so the IIDs might mean
-            # something different now. We simply forget our unused IIDs to
-            # avoid gc'ing foreign iterators.
-            # In the case that the server was not restarted, we accept the
-            # risk of leaking resources on the ZEO server.
-            pass
-        self._iterator_ids -= iids
+        if iids:
+            try:
+                self._server.iterator_gc(list(iids))
+            except ClientDisconnected:
+                # If we get disconnected, all of the iterators on the
+                # server are thrown away.  We should clear ours too:
+                return self._iterator_gc(True)
+            self._iterator_ids -= iids
 
 
 class TransactionIterator(object):
@@ -1409,6 +1416,9 @@
         if self._ended:
             raise ZODB.interfaces.StorageStopIteration()
 
+        if self._iid < 0:
+            raise ClientDisconnected("Disconnected iterator")
+
         tx_data = self._storage._server.iterator_next(self._iid)
         if tx_data is None:
             # The iterator is exhausted, and the server has already

Modified: ZODB/trunk/src/ZEO/tests/IterationTests.py
===================================================================
--- ZODB/trunk/src/ZEO/tests/IterationTests.py	2008-11-16 17:11:43 UTC (rev 93013)
+++ ZODB/trunk/src/ZEO/tests/IterationTests.py	2008-11-16 17:11:45 UTC (rev 93014)
@@ -99,3 +99,53 @@
         self.assertEquals(txn_info1.tid, txn_info2.tid)
         self.assertRaises(StopIteration, iter1.next)
         self.assertRaises(StopIteration, iter2.next)
+
+def iterator_sane_after_reconnect():
+    r"""Make sure that iterators are invalidated on disconnect.
+
+Start a server:
+
+    >>> addr, adminaddr = start_server(
+    ...     '<filestorage>\npath fs\n</filestorage>', keep=1)
+
+Open a client storage to it and commit a some transactions:
+
+    >>> import ZEO, transaction
+    >>> db = ZEO.DB(addr)
+    >>> conn = db.open()
+    >>> for i in range(10):
+    ...     conn.root().i = i
+    ...     transaction.commit()
+
+Create an iterator:
+
+    >>> it = conn._storage.iterator()
+    >>> tid1 = it.next().tid
+
+Restart the storage:
+
+    >>> stop_server(adminaddr)
+    >>> wait_disconnected(conn._storage)
+    >>> _ = start_server('<filestorage>\npath fs\n</filestorage>', addr=addr)
+    >>> wait_connected(conn._storage)
+
+Now, we'll create a second iterator:
+
+    >>> it2 = conn._storage.iterator()
+
+If we try to advance the first iterator, we should get an error:
+
+    >>> it.next().tid > tid1
+    Traceback (most recent call last):
+    ...
+    ClientDisconnected: Disconnected iterator
+
+The second iterator should be peachy:
+
+    >>> it2.next().tid == tid1
+    True
+
+Cleanup:
+
+    >>> db.close()
+    """

Modified: ZODB/trunk/src/ZEO/tests/testZEO.py
===================================================================
--- ZODB/trunk/src/ZEO/tests/testZEO.py	2008-11-16 17:11:43 UTC (rev 93013)
+++ ZODB/trunk/src/ZEO/tests/testZEO.py	2008-11-16 17:11:45 UTC (rev 93014)
@@ -1103,10 +1103,6 @@
     
 quick_test_classes = [FileStorageRecoveryTests, ConfigurationTests]
 
-def setUp(test):
-    ZODB.tests.util.setUp(test)
-    test.globs['get_port'] = lambda : get_port(test)
-
 def test_suite():
     suite = unittest.TestSuite()
 
@@ -1115,12 +1111,14 @@
     zeo = unittest.TestSuite()
     zeo.addTest(unittest.makeSuite(ZODB.tests.util.AAAA_Test_Runner_Hack))
     zeo.addTest(doctest.DocTestSuite(
-        setUp=setUp, tearDown=zope.testing.setupstack.tearDown))
+        setUp=forker.setUp, tearDown=zope.testing.setupstack.tearDown))
+    zeo.addTest(doctest.DocTestSuite(ZEO.tests.IterationTests,
+        setUp=forker.setUp, tearDown=zope.testing.setupstack.tearDown))
     zeo.addTest(doctest.DocFileSuite('registerDB.test'))
     zeo.addTest(
         doctest.DocFileSuite(
-            'zeo-fan-out.test',
-            setUp=setUp, tearDown=zope.testing.setupstack.tearDown,
+            'zeo-fan-out.test', 'zdoptions.test',
+            setUp=forker.setUp, tearDown=zope.testing.setupstack.tearDown,
             ),
         )
     for klass in quick_test_classes: