.

wcfs/client/wcfs.cpp

@@ -137,15 +137,11 @@
 //       client process
 //
 // This creates the necessity to use RWMutex for locks that pinner and other
-// parts of the code could be using at the same time in synchronous mode similar
-// to the above. This locks are:
+// parts of the code could be using at the same time in synchronous scenarious
+// similar to the above. This locks are:
 //
 //      - Conn.atMu
 //      - Conn.mu
-//
-//
-// XXX link to bigfile/file_zodb.cpp to show how wcfs/client is used for
-// ZBigFile on client side.
 
 
 #include "wcfs_misc.h"
@@ -204,10 +200,8 @@ pair<Conn, error> WCFS::connect(zodb::Tid at) {
         return wconn->_pinner(ctx);
     });
 
-    // NOTE no need to wait till `wcfs/head/at ≥ at` because Conn.open does it.
-    // FIXME ^^^ not right - Conn.open waits for wcfs/head/at only in the end
-    // and stats head/f to get f.headfsize _before_ that.
-    // -> just wait here.
+    // need to wait till `wcfs/head/at ≥ at` because e.g. Conn.open stats stats
+    // head/f to get f.headfsize.
     // XXX atMu.RLock ?
     err = wconn->_headWait(at);
     if (err != nil) {
@@ -458,24 +452,25 @@ error _Conn::resync(zodb::Tid at) {
     _Conn& wconn = *this;
     error err;
 
-    wconn._atMu.Lock();
+    wconn._atMu.RLock();
     xerr::Contextf E("%s: resync -> @%s", v(wconn), v(at));
-    wconn._atMu.Unlock();
+    wconn._atMu.RUnlock();
 
     // XXX downErr -> E
     // XXX at ^ (increases)
 
-    // first wait for wcfs/head to be >= at
+    // first wait for wcfs/head to be >= at.
+    // we need this e.g. to be sure that head/f.size is at least as big that it will be @at state.
     err = wconn._headWait(at);
     if (err != nil)
-        return err;
+        return err; // XXX -> wconn down on err ?
 
 
     // write-lock wconn._atMu . This excludes everything else, and in
     // particular _pinner_, from running and mutating files and mappings.
     //
     // NOTE we'll relock atMu as R in the second part of resync, so we prelock
-    // wconn._mu as well while under atMu.W, so that we know that set of opened
+    // wconn._mu.R as well while under atMu.W, to be sure that set of opened
     // files stays the same during whole resync.
     bool atMuWLocked = true;
     wconn._atMu.Lock();
@@ -500,17 +495,18 @@ error _Conn::resync(zodb::Tid at) {
     });
 
     // set new wconn.at early, so that e.g. Conn.open running simultaneously
-    // to second part of resync (see below) uses new at.    XXX no need since wconn._mu is locked?
+    // to second part of resync (see below) uses new at.
+    // XXX no need since wconn._mu is locked? -> no - it is *needed* after wconn.mu became RWMutex
     wconn.at = at;
 
-    // go through all files opened under wconn and pre-adjust files and their
-    // mappings for viewing data as of new @at state.
+    // go through all files opened under wconn and pre-adjust their mappings
+    // for viewing data as of new @at state.
     //
     // We are still holding atMu.W, so we are the only mutators of mappings,
     // because, in particular, pinner is not running.
     //
     // Don't send watch updates for opened files to wcfs yet - without running
-    // pinner those updates will be stuck.
+    // pinner those updates will get stuck.
     for (auto fit : wconn._filehTab) {
         //zodb::Oid  foid = fit.first;
         FileH        f    = fit.second;
@@ -518,6 +514,8 @@ error _Conn::resync(zodb::Tid at) {
         // TODO if file has no mappings and was not used during whole prev
         // cycle - forget and stop watching it
 
+        // XXX not yet ready f ?
+
         // update f._headfsize and remmap to head/f zero regions that are now covered by head/f
         struct stat st;
         err = f->_headf->stat(&st);
@@ -559,9 +557,10 @@ error _Conn::resync(zodb::Tid at) {
     // Now other calls, e.g. Conn.open, can be running simultaneously to us,
     // but since we already set wconn.at to new value it is ok. For example
     // Conn.open, for not-yet-opened file, will use new at to send "watch".
-    // XXX ^^^ not possible since wconn._mu is locked ? -> no, possible, wconn._mu became RWMutex
+    // XXX ^^^ not possible since wconn._mu is locked ?
+    //     -> no, possible, wconn._mu became RWMutex
     //
-    // XXX we are still holding wconn._mu locked, so wconn._filehTab is the
+    // XXX we are still holding wconn._mu.R, so wconn._filehTab is the
     // same as in previous pass above.
     wconn._atMu.UnlockToRLock();
     atMuWLocked = false;
@@ -625,6 +624,9 @@ pair<FileH, error> _Conn::open(zodb::Oid foid) {
     f->foid       = foid;
     f->_openReady = makechan<structZ>();
     f->_openErr   = nil;
+    f->_headf     = nil;
+    f->blksize    = 0;
+    f->_headfsize = 0;
 
     bool retok = false;
     wconn._filehTab[foid] = f;
@@ -674,7 +676,7 @@ error _FileH::_open() {
     if (err != nil)
         return err;
     f->blksize    = st.st_blksize;
-    f->_headfsize = st.st_size; // FIXME getting headfsize _before_ waiting for wcfs/head/at ≥ wconn.at
+    f->_headfsize = st.st_size;
     if (!(f->_headfsize % f->blksize == 0))
         return fmt::errorf("wcfs bug: %s size (%d) %% blksize (%d) != 0",
                         v(f->_headf->name()), f->_headfsize, f->blksize);

wcfs/client/wcfs.h

@@ -181,7 +181,6 @@ struct _Conn : object {
     sync::RWMutex _atMu;
     zodb::Tid     at;
 
-//  sync::Mutex             _mu;       // _atMu.W  |  _atMu.R + _mu
     sync::RWMutex           _mu;       // _atMu.W  |  _atMu.R + _mu
     error                   _downErr;  // !nil if connection is closed or no longer operational
     dict<zodb::Oid, FileH>  _filehTab; // {} foid -> fileh