.

wcfs/client/wcfs.cpp

@@ -142,6 +142,19 @@
 //
 //      - Conn.atMu
 //      - Conn.mu
+//
+// XXX pinner takes the following locks (XXX recheck)
+//
+//      - wconn.mu.W
+//      - wconn.mu.R
+//
+//      - wconn.atMu.R
+//      - wconn.mu.R
+//      - fileh.mu (R:.mmaps W:.pinned)
+//
+//      - virt_lock
+//
+//
 
 
 #include "wcfs_misc.h"
@@ -555,7 +568,7 @@ error _Conn::resync(zodb::Tid at) {
     // - we need atMu to be not Wlocked, because under atMu.W pinner cannot run simultaneously to us.
     // - we need to hold atMu.R to avoid race wrt e.g. other resync which changes at.
     // - we cannot just do regular `atMu.Unlock + atMu.RLock()` because then
-    //   there is a race window in between Unlock and RLock where wconn.at can be changed.
+    //   there is e.g. a race window in between Unlock and RLock where wconn.at can be changed.
     //   XXX also deadlock, because it will become wconn._mu.lock + wconn._atMu lock
     //
     // Now other calls, e.g. Conn.open, can be running simultaneously to us,
@@ -576,7 +589,10 @@ error _Conn::resync(zodb::Tid at) {
         //FileH    f    = fit.second;
 
         // XXX need to lock f.mu because wconn.atMu is only R now.
+        // XXX need to coordinate with e.g. FileH.close
         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 @%s", v(foid), v(at)));
         if (err != nil)
             return E(err);
@@ -605,8 +621,9 @@ pair<FileH, error> _Conn::open(zodb::Oid foid) {
     wconn._mu.Lock();
 
     if (wconn._downErr != nil) {
+        err = wconn._downErr;
         wconn._mu.Unlock();
-        return make_pair(nil, E(wconn._downErr));
+        return make_pair(nil, E(err));
     }
 
     FileH f; bool ok;
@@ -695,6 +712,8 @@ error _FileH::_open() {
     // NOTE we are _not_ holding wconn.mu nor f.mu - only wconn.atMu to rely on wconn.at being stable.
     // NOTE wcfs will reply "ok" only after wcfs/head/at ≥ wconn.at
     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 @%s", v(foid), v(wconn->at)));
     if (err != nil)
         return err;
@@ -744,6 +763,8 @@ error _FileH::close() {
 
     // stop watching f          XXX ok under f.mu ?
     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);
@@ -752,8 +773,9 @@ error _FileH::close() {
 
     // remove fileh from wconn._filehTab
     wconn->_mu.Lock();  // FIXME lock order vs fileh._mu
-    if (wconn->_filehTab.get(fileh.foid)._ptr() == &fileh) // XXX -> panic(BUG)
-        wconn->_filehTab.erase(fileh.foid);
+    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());

wcfs/client/wcfs.h

@@ -243,6 +243,9 @@ struct _FileH : object {
     int                       _nopen;  // number of times Conn.open returned this fileh
     bool                      _closed; // y after .close()
 
+    // "watch <.foid> ..." requests we send to wcfs are serialized via FileH._watchMu
+    // XXX sync::Mutex        _watchMu;
+
     // don't new - create via Conn.open
 private:
     _FileH();