Add a IFileIO::close() to help catch NFS write errs

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

common/remote/sockfile.cpp

@@ -2195,24 +2195,11 @@ public:
         disconnectonexit = false;
     }
 
-
     ~CRemoteFileIO()
     {
         if (handle) {
             try {
-                MemoryBuffer sendBuffer;
-                initSendBuffer(sendBuffer);
-                sendBuffer.append((RemoteFileCommandType)RFCcloseIO).append(handle);
-                parent->sendRemoteCommand(sendBuffer,false);
-            }
-            catch (IDAFS_Exception *e) {
-                if ((e->errorCode()!=RFSERR_InvalidFileIOHandle)&&(e->errorCode()!=RFSERR_NullFileIOHandle)) { // ignore already disconnected
-                    StringBuffer s;
-                    e->errorMessage(s);
-                    WARNLOG("CRemoteFileIO close file: %s",s.str());
-                }
-                e->Release();
-                
+                close();
             }
             catch (IException *e) {
                 StringBuffer s;
@@ -2223,7 +2210,24 @@ public:
         }
         if (disconnectonexit)
             parent->disconnect();
-        handle = 0;
+    }
+
+    void close()
+    {
+        if (handle) {
+            try {
+                MemoryBuffer sendBuffer;
+                initSendBuffer(sendBuffer);
+                sendBuffer.append((RemoteFileCommandType)RFCcloseIO).append(handle);
+                parent->sendRemoteCommand(sendBuffer,false);
+            }
+            catch (IDAFS_Exception *e) {
+                if ((e->errorCode()!=RFSERR_InvalidFileIOHandle)&&(e->errorCode()!=RFSERR_NullFileIOHandle))
+                    throw;
+                e->Release();
+            }
+            handle = 0;
+        }
     }
 
     bool open(IFOmode _mode,compatIFSHmode _compatmode) 

dali/datest/datest.cpp

@@ -2349,6 +2349,7 @@ NULL
         virtual offset_t appendFile(IFile *file,offset_t pos=0,offset_t len=-1) { UNIMPLEMENTED; return 0; }
         virtual void setSize(offset_t size) { UNIMPLEMENTED; }
         virtual void flush() { }
+        virtual void close() { }
     };
 
     const char *newFileName = "xpathTests.out";

roxie/ccd/ccdfile.cpp

@@ -71,6 +71,7 @@ public:
     virtual size32_t write(offset_t pos, size32_t len, const void * data) { THROWNOTOPEN; }
     virtual void setSize(offset_t size) { UNIMPLEMENTED; }
     virtual offset_t appendFile(IFile *file,offset_t pos,offset_t len) { UNIMPLEMENTED; return 0; }
+    virtual void close() { }
 } failure;
 
 class CLazyFileIO : public CInterface, implements ILazyFileIO, implements IDelayedFile

system/jlib/jfile.cpp

@@ -1546,6 +1546,7 @@ IFileIO *_createIFileIO(const void *buffer, unsigned sz, bool readOnly)
             return len;
         }
         virtual void flush() {}
+        virtual void close() {}
         virtual void setSize(offset_t size)
         {
             if (size > mb.length())
@@ -1737,7 +1738,26 @@ CFileIO::CFileIO(HANDLE handle, IFSHmode _sharemode)
 
 CFileIO::~CFileIO()
 {
-    if (file != NULLFILE) CloseHandle(file);
+    try
+    {
+        //note this will not call the virtual close() if anyone ever derived from this class.
+        //the clean fix is to move this code to beforeDispose()
+        close();
+    }
+    catch (IException * e)
+    {
+        EXCLOG(e, "CFileIO::~CFileIO");
+        e->Release();
+    }
+}
+
+void CFileIO::close()
+{
+    if (file != NULLFILE)
+    {
+        if (!CloseHandle(file))
+            throw MakeOsException(GetLastError(),"CFileIO::close");
+    }
     file = NULLFILE;
 }
 
@@ -1815,8 +1835,22 @@ CFileIO::CFileIO(HANDLE handle, IFSHmode _sharemode)
 
 CFileIO::~CFileIO()
 {
+    try
+    {
+        close();
+    }
+    catch (IException * e)
+    {
+        EXCLOG(e, "CFileIO::~CFileIO");
+        e->Release();
+    }
+}
+
+void CFileIO::close()
+{
     if (file != NULLFILE) {
-        close(file);
+        if (::close(file) < 0)
+            throw MakeErrnoException(errno,"CFileIO::close");
         file=NULLFILE;
     }
 }
@@ -1906,22 +1940,26 @@ size32_t CFileRangeIO::write(offset_t pos, size32_t len, const void * data)
 
 //--------------------------------------------------------------------------
 
-void CFileAsyncIO::flush()
+CFileAsyncIO::~CFileAsyncIO()
 {
-    //This could wait until all pending results are done.
-    loop
+    try
     {
-        Owned<IFileAsyncResult> next;
-        {
-            CriticalBlock block(cs);
-            if (results.ordinality())
-                next.set(&results.tos());
-        }
-        if (!next)
-            return;
+        close();
+    }
+    catch (IException * e)
+    {
+        EXCLOG(e, "CFileAsyncIO::~CFileAsyncIO");
+        e->Release();
+    }
+}
 
-        size32_t value;
-        next->getResult(value, true);
+void CFileAsyncIO::flush()
+{
+    // wait for all outstanding results
+    CriticalBlock block(cs);
+    ForEachItemInRev(i,results) {
+        size32_t dummy;
+        results.item(i).getResult(dummy,true);
     }
 }
 
@@ -1999,15 +2037,15 @@ CFileAsyncIO::CFileAsyncIO(HANDLE handle, IFSHmode _sharemode)
     sharemode = _sharemode;
 }
 
-CFileAsyncIO::~CFileAsyncIO()
+void CFileAsyncIO::close()
 {
-    CriticalBlock block(cs);
-    ForEachItemInRev(i,results) {
-        size32_t dummy;
-        results.item(i).getResult(dummy,true);
-    }
+    flush();
     // wait for all outstanding results
-    if (file != NULLFILE) CloseHandle(file);
+    if (file != NULLFILE)
+    {
+        if (!CloseHandle(file))
+            throw MakeOsException(GetLastError(),"CFileAsyncIO::close");
+    }
     file = NULLFILE;
 }
 
@@ -2169,11 +2207,12 @@ CFileAsyncIO::CFileAsyncIO(HANDLE handle, IFSHmode _sharemode)
 }
 
 
-CFileAsyncIO::~CFileAsyncIO()
+void CFileAsyncIO::close()
 {
     if (file != NULLFILE) {
         aio_cancel(file,NULL);
-        _lclose(file);
+        if (_lclose(file) < 0)
+            throw MakeErrnoException(errno, "CFileAsyncIO::close");
     }
     file=NULLFILE;
 }
@@ -6138,6 +6177,15 @@ public:
         if (cachedio)
             cachedio->flush();
     }
+    virtual void close()
+    {
+        CriticalBlock block(sect);
+        if (cachedio)
+        {
+            cachedio->close();
+            cachedio.clear();
+        }
+    }
     offset_t appendFile(IFile *file,offset_t pos,offset_t len)
     {
         CriticalBlock block(sect);

system/jlib/jfile.hpp

@@ -165,6 +165,7 @@ interface IFileIO : public IInterface
     virtual offset_t appendFile(IFile *file,offset_t pos=0,offset_t len=(offset_t)-1) =0;
     virtual void setSize(offset_t size) = 0;
     virtual void flush() = 0;
+    virtual void close() = 0;       // no other access is allowed after this call
 };
 
 interface IFileIOCache : extends IInterface

system/jlib/jfile.ipp

@@ -107,6 +107,7 @@ public:
     virtual void setSize(offset_t size);
     virtual offset_t appendFile(IFile *file,offset_t pos,offset_t len);
     virtual void flush();
+    virtual void close();
 
     bool create(const char * filename, bool replace);
     bool open(const char * filename);
@@ -136,6 +137,7 @@ public:
     virtual void setSize(offset_t size) { UNIMPLEMENTED; }
     virtual offset_t appendFile(IFile *file,offset_t pos,offset_t len) { UNIMPLEMENTED; return 0; }
     virtual void flush() { io->flush(); }
+    virtual void close() { io->close(); }
 
 protected:
     Linked<IFileIO>     io;
@@ -157,6 +159,7 @@ public:
     virtual size32_t write(offset_t pos, size32_t len, const void * data);
     virtual offset_t appendFile(IFile *file,offset_t pos,offset_t len);
     virtual void flush();
+    virtual void close();
 
     virtual void setSize(offset_t size);
     virtual IFileAsyncResult *readAsync(offset_t pos, size32_t len, void * data);

thorlcr/activities/thactivityutil.cpp

@@ -825,6 +825,7 @@ public:
     virtual offset_t appendFile(IFile *file,offset_t pos=0,offset_t len=-1) { return primaryio->appendFile(file, pos, len); }
     virtual void setSize(offset_t size) { primaryio->setSize(size); }
     virtual void flush() { primaryio->flush(); }
+    virtual void close() { primaryio->close(); }
 };
 
 IFileIO *createMultipleWrite(CActivityBase *activity, IPartDescriptor &partDesc, unsigned recordSize, bool &compress, bool extend, ICompressor *ecomp, ICopyFileProgress *iProgress, bool direct, bool renameToPrimary, bool *aborted, StringBuffer *_outLocationName)

thorlcr/graph/thgraphslave.cpp

@@ -1172,12 +1172,6 @@ public:
         iFileIO.clear();
     }
 
-    void close()
-    {
-        CriticalBlock b(crit);
-        iFileIO.clear();
-    }
-
     const char *queryFindString() const { return filename.get(); } // for string HT
 
 // IFileIO impl.
@@ -1205,6 +1199,13 @@ public:
         if (iFileIO)
             iFileIO->flush();
     }
+    virtual void close()
+    {
+        CriticalBlock b(crit);
+        if (iFileIO)
+            iFileIO->close();
+        iFileIO.clear();
+    }
     virtual offset_t appendFile(IFile *file,offset_t pos=0,offset_t len=(offset_t)-1)
     {
         CriticalBlock b(crit);