.

wcfs/client/wcfs.cpp

@@ -638,7 +638,7 @@ retry:
         bool closing;
 
         if (f->_state <= _FileHOpened) {
-            f->_nopen++; // XXX lock by f.mu ?
+            f->_nopen++;
             closing = false;
         } else {
             closing = true;
@@ -759,11 +759,10 @@ error _FileH::close() {
     _FileH& fileh = *this;
     Conn    wconn = fileh.wconn;
 
-    // XXX locking ok?
     wconn->_atMu.RLock();
-    fileh._mu.lock();       // XXX for fileh ._nopen, ._closed  ->better use wconn._mu for those too?
+    wconn->_mu.Lock()
     defer([&]() {
-        fileh._mu.unlock();
+        wconn->_mu.Unlock();
         wconn->_atMu.RUnlock();
     });
 
@@ -784,37 +783,38 @@ error _FileH::close() {
     ASSERT(fileh._state == _FileHOpened); // there can be no open-in-progress, because
     fileh._state = _FileHClosing;         // .close() can be called only on "opened" fileh
 
+    // unlock wconn.mu to stop watching outside of this lock.
+    // we'll relock it again before updating wconn.filehTab.
+    wconn->_mu.Unlock();
+
+
     error err, eret;
     auto reterr1 = [&eret](error err) {
         if (eret == nil && err != nil)
             eret = err;
     };
 
+    // XXX do something with f.mu ?
     // XXX change all fileh.mmaps to cause EFAULT on any access after fileh.close
 
-    // XXX unlock wconn.mu
-    // XXX unlock f.mu
-
-    // stop watching f          XXX ok under f.mu ?
+    // stop watching f
     string ack;
-    // XXX f._watchMu.lock() + unlock
-    // XXX + recheck status before sending the watch?
     tie(ack, err) = wconn->_wlink->sendReq(context::background(), fmt::sprintf("watch %s -", v(foid)));
     if (err != nil)
         reterr1(err);
     else if (ack != "ok")
         reterr1(fmt::errorf("unwatch: %s", v(ack)));
 
-    // remove fileh from wconn._filehTab
-    wconn->_mu.Lock();  // FIXME lock order vs fileh._mu
+
+    // relock wconn._mu again and remove fileh from wconn._filehTab
+    wconn->_mu.Lock();
     if (wconn->_filehTab.get(fileh.foid)._ptr() != &fileh)
         panic("BUG: fileh.close: wconn.filehTab[fileh.foid] != fileh");
     wconn->_filehTab.erase(fileh.foid);
-    wconn->_mu.Unlock();
 
     reterr1(fileh._headf->close());
 
-    fileh._state = _FileHClosed; // XXX locking
+    fileh._state = _FileHClosed;
     fileh._closedq.close();
 
     return E(eret);

wcfs/client/wcfs.h

@@ -181,7 +181,7 @@ struct _Conn : object {
     sync::RWMutex _atMu;
     zodb::Tid     at;
 
-    sync::RWMutex           _mu;       // _atMu.W  |  _atMu.R + _mu     XXX -> _filehMu ?
+    sync::RWMutex           _mu;       // _atMu.W  |  _atMu.R + _mu     XXX -> _filehMu ? -> y
     error                   _downErr;  // !nil if connection is closed or no longer operational
     dict<zodb::Oid, FileH>  _filehTab; // {} foid -> fileh
 
@@ -234,7 +234,6 @@ struct _FileH : object {
     // protect by wconn.mu
     _FileHState _state;  // opening/opened/closing/closed
     int         _nopen;  // number of times Conn.open returned this fileh
-    bool        _closed; // y after .close()
 
     chan<structZ> _openReady; // in-flight open completed
     error         _openErr;   // error result from open