Hi Andy, Matevz,

Thanks for considering the patch. Sorry I think I didn't my goal well at first: my idea is not to allow XrdPosixXrootd::Close() (and hence Detach()) to be called multiple times.

I was in the situation where the first and only call of fP->Close(Status) on line XrdPosixXrootd.cc:288 fails. The file is then entered for Delayed destroy, I believe as intended, at least it was it was doing, and XrdPosixFile::DelayedDestroy() then periodically considers closing the file at line XrdPosixFile.cc:224 up to XrdPosixGlobals::ddMaxTries (6) times. There is a question of whether it will ever succeed - if not then the current action of leaving the file lost seems the best that can be done - but in my case it was possible that the retry would work.

What I saw in my test was that the Close() from line XrdPosixFile.cc:224 is never attempted (seen by instrumenting XrdCl). I understand the reason for that is that the condition at line XrdPosixFile.cc:223 is not true, i.e. Refs() is not zero, but = 1. I don't think anything was using the file.

So my idea with the patch was to make sure that the ref count of the file will eventually become zero (assuming that nothing is using the file). It seemed to me this didn't work with the current code when Detach() returned true (i.e. synchronous, and so never calling the callback which would have done an unRef() itself) .

Andy suggested:

fP->Ref();
if (fP->XCio->Detach((XrdOucCacheIOCD&)*fP)) fP->Unref();
if (fP->Refs() < 2)

I think there's a problem in that the 3rd line, Refs()<2, is no longer the right condition. The believe that the condition wanted is that Detach() was synchronous and nothing is now using the file. Avoiding adding another variable, perhaps:

ret = true;
fP->Ref();
if (fP->XCio->Detach((XrdOucCacheIOCD&)*fP)) {
   fP->unRef();
   if (fP->Refs() == 0) {
      if ((ret = fP->Close(Status))) {delete fP; fP = 0;}
         else if (DEBUGON)
                     {std::string eTxt = Status.ToString();
                      DEBUG(eTxt <<" closing " <<fP->Origin());
                      }
   }
}

// ....
if (fP) XrdPosixFile::DelayedDestroy(fP);

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <xrootd/xrootd/pull/1724/c1161508253@github.com>

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1