.

wcfs/wcfs.go

@@ -977,11 +977,11 @@ func (f *BigFile) invalidateBlk(ctx context.Context, blk int64) (err error) {
 		func() {
 			// store retrieved data back to OS cache for file @<rev>/file[blk]
 			blkrev, _ := f.LastBlkRev(ctx, blk, f.head.zconn.At())
-			frev, frelease, err := groot.mkrevfile(blkrev, f.zfile.POid())
+			frev, funlock, err := groot.lockRevFile(blkrev, f.zfile.POid())
 			if err != nil {
 				log.Errorf("BUG: %s: invalidate blk #%d: %s (ignoring, but reading @revX/bigfile will be slow)", f.path(), blk, err)
 			}
-			defer frelease()
+			defer funlock()
 
 			st := fsconn.FileNotifyStoreCache(frev.Inode(), off, blkdata)
 			if st != fuse.OK {
@@ -1014,27 +1014,34 @@ func (f *BigFile) invalidateAttr() (err error) {
 }
 
 
-// mkrevfile makes sure inode ID of /@<rev>/bigfile/<fid> is known to kernel.
+// lockRevFile makes sure inode ID of /@<rev>/bigfile/<fid> is known to kernel
+// and won't change until unlock.
 //
 // We need node ID to be know to the kernel, when we need to store data into
 // file's kernel cache - if the kernel don't have the node ID for the file in
 // question, FileNotifyStoreCache will just fail.
 //
-// For kernel to know the inode mkrevfile issues regular filesystem lookup
+// For kernel to know the inode lockRevFile issues regular filesystem lookup
 // request which goes to kernel and should go back to wcfs. It is thus not safe
-// to use mkrevfile from under FUSE request handler as doing so might deadlock.
+// to use lockRevFile from under FUSE request handler as doing so might deadlock.
 //
-// Caller must call release when inode ID is no longer required to be present.
-func (root *Root) mkrevfile(rev zodb.Tid, fid zodb.Oid) (_ *BigFile, release func(), err error) {
+// Caller must call unlock when inode ID is no longer required to be present.
+// It is safe to simultaneously call multiple lockRevFile with the same arguments.
+func (root *Root) lockRevFile(rev zodb.Tid, fid zodb.Oid) (_ *BigFile, unlock func(), err error) {
 	fsconn := gfsconn
 
 	frevpath := fmt.Sprintf("@%s/bigfile/%s", rev, fid) // relative to fs root for now
-	defer xerr.Contextf(&err, "/: mkrevfile %s", frevpath)
+	defer xerr.Contextf(&err, "/: lockRevFile %s", frevpath)
 
 	// FIXME checking for "node{0}" is fragile:
 	// XXX the node could be still forgotten since we are not holding open on it
 	// XXX -> always os.open unconditionally for now
 	//        or is it ok since it is just a cache?
+	//	  -> no, not ok: if inode ID is forgotten, the same ID could be
+	//	     reallocated to another file and then we'll corrupt in-kernel
+	//	     cache by wrongly storing data of one file into cache of
+	//	     another file.
+	//	     -> to avoid this we need to always lock the inode ID with real open.
 	// XXX (also disabled for now due to race-detector)
 /*
 	// first check without going through kernel, whether the inode maybe known already