X wcfs: Fix ZSync to close wconn on zdb.close, even if zconn stays alive

Even if ZODB DB is closed and releases zconn from its .pool, zconn can
still stay alive being referenced from any other live python object -
e.g. from a frame, a traceback etc. This situation in particular happens
under ERP5's runUnitTest.

As the result, if we don't hook into DB.close and close wconn only on
zconn GC, files opened on WCFS filesystem stay opened, and WCFS server
cannot be cleanly unmounted when test run completes.

-> Fix it.

P.S. ZODB@bbd03b3a is good, but does
not really solve this problem because, once again, zconn stays
referenced from objects besides DB.

wcfs/client/_wczsync.pyx

@@ -58,7 +58,7 @@ cdef wcfs.PyConn pywconnOf(zconn):
     zconn._wcfs_wconn = wconn
 
     # keep wconn view of the database in sync with zconn
-    # wconn and wc (= wconn.wc) will be closed when zconn is garbage-collected
+    # wconn and wc (= wconn.wc) will be closed when zconn is garbage-collected or shutdown via DB.close
     _ZSync(zconn, wconn)
 
     return wconn
@@ -66,8 +66,8 @@ cdef wcfs.PyConn pywconnOf(zconn):
 
 # _ZSync keeps wconn in sync with zconn.
 #
-# wconn will be closed once zconn is destroyed (not closed, which returns it
-# back into DB pool).
+# wconn will be closed once zconn is garbage-collected (not closed, which
+# returns it back into DB pool), or once zconn.db is closed.
 #
 # _ZSync cares itself to stay alive as long as zconn stays alive.
 _zsyncReg   = {} # id(zsync) -> zsync   (protected by GIL)
@@ -79,8 +79,12 @@ class _ZSync:
         #print('ZSync: setup %r <-> %r' % (wconn, zconn))
         assert zconn.opened
         zsync.wconn     = wconn
+        # notify us on zconn GC
         zsync.zconn_ref = weakref.ref(zconn, zsync.on_zconn_dealloc)
+        # notify us on zconn.db.close
+        zconn.onShutdownCallback(zsync)
 
+        # notify us when zconn changes its view of the database
         # NOTE zconn.onOpenCallback is not enough: zconn.at can change even
         # without zconn.close/zconn.open, e.g.:
         # zconn = DB.open(transaction_manager=tm)
@@ -98,8 +102,14 @@ class _ZSync:
         if 1: # = `with gil:`  (GIL already held in python code)
             _zsyncReg[id(zsync)] = zsync
 
-    # .zconn dealloc -> wconn.close; release zsync
-    def on_zconn_dealloc(zsync, _):
+    # _release1 closes .wconn and releases zsync once.
+    def _release1(zsync):
+        # unregister zsync from being kept alive
+        if 1: # = `with gil:`  (see note in __init__)
+            _ = _zsyncReg.pop(id(zsync), None)
+        if _ is None:
+            return # another call already done/is simultaneously doing release1
+
         #print('ZSync: sched break %r <-> .' % (zsync.wconn,))
         # schedule wconn.close() + wconn.wc.close()
         _zsync_wclose_wg.add(1)
@@ -112,9 +122,14 @@ class _ZSync:
         _zsync_releaseq.send(zsync.wconn)
         """
 
-        # unregister zsync from being kept alive
-        if 1: # = `with gil:`  (see note in __init__)
-            del _zsyncReg[id(zsync)]
+
+    # .zconn dealloc -> wconn.close; release zsync.
+    def on_zconn_dealloc(zsync, _):
+        zsync._release1()
+
+    # DB.close tells us that .zconn is shut down  -> wconn.close; release zsync.
+    def on_connection_shutdown(zsync):
+        zsync._release1()
 
     # DB resyncs .zconn onto new database view.
     # -> resync .wconn to updated database view of ZODB connection.

wcfs/client/_wczsync_test.py

@@ -39,12 +39,10 @@ def setup_module():
 def teardown_module():
     testdb.teardown()
 
-# verify that ZSync keeps wconn in sync wrt zconn.
-@func
-def test_zsync():
-    zstor = testdb.getZODBStorage()
-    defer(zstor.close)
 
+# _zsync_setup setups up DB, zconn and wconn _ZSync'ed to zconn.
+@func
+def _zsync_setup(zstor): # -> (db, zconn, wconn)
     zurl = zstor_2zurl(zstor)
 
     # create new DB that we'll precisely control
@@ -53,7 +51,6 @@ def test_zsync():
     at0 = zconn_at(zconn)
     # create wconn
     wc = wcfs.join(zurl)
-    wc_njoin0 = wc._njoin
     wconn = wc.connect(at0)
     assert wconn.at() == at0
     # setup ZSync for wconn <-> zconn; don't keep zsync explicitly referenced
@@ -61,8 +58,63 @@ def test_zsync():
     _ZSync(zconn, wconn)
 
     assert wconn.at() == at0
+    return db, zconn, wconn
+
+
+# verify that ZSync closes wconn when db is closed.
+@func
+def test_zsync_db_close():
+    zstor = testdb.getZODBStorage()
+    defer(zstor.close)
+
+    db, zconn, wconn = _zsync_setup(zstor)
+    defer(wconn.close)
+
+    # close db -> ZSync should close wconn and wc even though zconn stays referenced
+    wc_njoin0 = wconn.wc._njoin
+    db.close()
+    _zsync_wclose_wg.wait()
+    # NOTE db and zconn are still alive - not GC'ed
+    with raises(error, match=": connection closed"):
+        wconn.open(p64(0))
+    assert wconn.wc._njoin == (wc_njoin0 - 1)
+
+
+# verify that ZSync closes wconn when zconn is garbage-collected.
+@func
+def test_zsync_zconn_gc():
+    zstor = testdb.getZODBStorage()
+    defer(zstor.close)
+
+    db, zconn, wconn = _zsync_setup(zstor)
+    defer(wconn.close)
+
+    # del zconn/db -> zconn should disappear and ZSync should close wconn and wc
+    zconn_weak = weakref.ref(zconn)
+    assert zconn_weak() is not None
+    wc_njoin0 = wconn.wc._njoin
+    del zconn
+    del db
+    gc.collect()
+    assert zconn_weak() is None
+    _zsync_wclose_wg.wait()
+    with raises(error, match=": connection closed"):
+        wconn.open(p64(0))
+    assert wconn.wc._njoin == (wc_njoin0 - 1)
+
+
+# verify that ZSync keeps wconn in sync wrt zconn.
+@func
+def test_zsync_resync():
+    zstor = testdb.getZODBStorage()
+    defer(zstor.close)
+
+    db, zconn, wconn = _zsync_setup(zstor)
+    defer(db.close)
 
     # commit something - ZSync should resync wconn to updated db state
+    at0 = zconn_at(zconn)
+    assert wconn.at() == at0
     root = zconn.root()
     root['tzync'] = 1
     transaction.commit()
@@ -97,13 +149,3 @@ def test_zsync():
     assert zconn_weak() is zconn
     assert zconn_at(zconn) == at2
     assert wconn.at() == at2
-
-    # close db -> zconn should disappear and ZSync should close wconn and wc
-    del zconn
-    db.close()
-    gc.collect()
-    assert zconn_weak() is None
-    _zsync_wclose_wg.wait()
-    with raises(error, match=": connection closed"):
-        wconn.open(p64(0))
-    assert wc._njoin == wc_njoin0 - 1