.

wcfs/internal/wcfs.cpp

@@ -108,8 +108,7 @@ static global<error> errConnClosed = errors::New("connection closed");
 
 // close releases resources associated with wconn.
 //
-// opened fileh and mappings becomes invalid to use.
-// XXX what happens to file mmappings?
+// opened fileh and mappings becomes invalid to use except close and unmap.
 error _Conn::close() {
     _Conn& wconn = *this;
     //xerr::Contextf E("close conn @%s", v(wconn.at));
@@ -135,9 +134,10 @@ error _Conn::close() {
         reterr1(err);
 
     // close all files - both that have no mappings and that still have opened mappings.
-    // XXX after file is closed mappings continue to survive, but we can no
-    // longer maintain consistent view.
-    // XXX change mapping to something that gives EFAULT on access?
+    //
+    // NOTE after file is closed mappings could continue to survive, but we can no
+    // longer maintain consistent view. For this reason we change mappings to
+    // something that gives EFAULT on access. XXX implement
     wconn._filehmu.lock();
     defer([&]() {
         wconn._filehmu.unlock();
@@ -145,7 +145,7 @@ error _Conn::close() {
 
     for (auto _ : wconn._filehtab) {
         auto f = _.second;
-        err = f->_headf->close(); // XXX better -> fileh.close()? or deadlock on ._filehmu ?
+        err = f->_headf->close(); // XXX mark fileh as down so that fileh.close does not say "bad fd"
         if (err != nil)
             reterr1(err);
         f->_headf = nil;
@@ -302,13 +302,13 @@ pair<FileH, error> _Conn::open(zodb::Oid foid) {
         wconn._filehmu.unlock();
     });
 
-    if (wconn._downErr != nil)
+    if (wconn._downErr != nil)  // XXX under filehmu or downMu ?
         return make_pair(nil, E(wconn._downErr));
 
     FileH f; bool ok;
     tie(f, ok) = wconn._filehtab.get(foid);
     if (f != nil) {
-        // XXX incref open count?
+        // XXX incref open count
         return make_pair(f, nil);
     }
 
@@ -349,12 +349,20 @@ pair<FileH, error> _Conn::open(zodb::Oid foid) {
 
 // close releases resources associated with FileH.
 //
-// Left fileh mappings become invalid to use.
+// Left fileh mappings become invalid to use except unmap.
 error _FileH::close() {
     _FileH& fileh = *this;
-    xerr::Contextf E("conn @%s: close f<%s>", v(fileh.wconn->at), v(fileh.foid)); // XXX + wcfs path?
-
-    // XXX remove fileh from .wconn._filehtab ?
+    Conn    wconn = fileh.wconn;
+    xerr::Contextf E("conn @%s: close f<%s>", v(wconn->at), v(fileh.foid)); // XXX + wcfs path?
+
+    // remove fileh from wconn._filehtab
+    // fileh.close can be called several times and after first call another
+    // fileh could be opened for the same foid. Be careful not to erase it.
+    wconn->_filehmu.lock();
+    // XXX decref open count
+    if (wconn->_filehtab.get(fileh.foid) == fileh)
+        wconn->_filehtab.erase(fileh.foid);
+    wconn->_filehmu.unlock();
 
     return E(fileh._headf->close());
 }

wcfs/internal/wcfs.h

@@ -104,6 +104,7 @@ struct _Conn : object {
     zodb::Tid   at;
     WatchLink   _wlink; // watch/receive pins for mappings created under this conn
 
+    // XXX kill downMu? (move under filehmu so that e.g. .open() can check downErr without race)
     sync::Mutex _downMu;
     error       _downErr; // !nil if connection is closed or no longer operational
 

wcfs/wcfs_test.py

@@ -1820,6 +1820,7 @@ def test_wcfs_virtmem():
     # XXX w mapping with RW - in sync
 
     # XXX fh close then open again and use
+    # XXX open same fh twice, close once - fh2 continue to work ok
 
     # XXX wconn.open()      after wconn.close() -> error
     # XXX wconn.resync()    after wconn.close() -> error