HPCC-32553 Ensure errors are reported when closing files

Signed-off-by: Gavin Halliday <[email protected]>

dali/base/dacoven.cpp

@@ -563,12 +563,14 @@ class CCovenServer: public CCovenBase
         Owned<IFile> f = createIFile(storename.get());
         Owned<IFileIO> io = f->open(IFOcreate);
         io->write(0, xml.length(), xml.str());
+        io->close();
         io.clear();
         if (!backupname.isEmpty()) {
             try {
                 f.setown(createIFile(backupname.get()));
                 io.setown(f->open(IFOcreate));
                 io->write(0, xml.length(), xml.str());
+                io->close();
                 io.clear();
             }
             catch (IException *e) {

dali/base/dasds.cpp

@@ -1022,6 +1022,7 @@ void writeDelta(StringBuffer &xml, IFile &iFile, const char *msg="", unsigned re
             sprintf(strNum, "%016" I64F "X", fLen);
             memcpy(headerPtr + deltaHeaderSizeOff, strNum, 16);
             iFileIO->write(0, strlen(deltaHeader), headerPtr);
+            iFileIO->close();
         }
         catch (IException *e)
         {
@@ -1213,6 +1214,7 @@ class CDeltaWriter : implements IThreaded
                 Owned<IFile> iFile = createIFile(rL.str());
                 Owned<IFileIO> fileIO = iFile->open(IFOcreate);
                 fileIO->write(0, length, data);
+                fileIO->close();
             }
             catch (IException *e)
             {

dali/server/daserver.cpp

@@ -612,6 +612,7 @@ int main(int argc, const char* argv[])
                     backupCheck.append("bakchk.").append((unsigned)GetCurrentProcessId());
                     OwnedIFile iFileDataDir = createIFile(backupCheck.str());
                     OwnedIFileIO iFileIO = iFileDataDir->open(IFOcreate);
+                    iFileIO->close();
                     iFileIO.clear();
                     try
                     {

ecl/ecl-bundle/ecl-bundle.cpp

@@ -750,6 +750,7 @@ class CBundleInfoSet : public CInterfaceOf<IBundleInfoSet>
         VStringBuffer redirect("IMPORT %s.%s.%s.%s as _%s; EXPORT %s := _%s;", VERSION_SUBDIR, name, version, name, name, name, name);
         Owned<IFileIO> rfile = redirector->open(IFOcreate);
         rfile->write(0, redirect.length(), redirect.str());
+        rfile->close();
         bundle->setActive(true);
     }
     virtual void setActive(const char *version)

ecl/eclcc/eclcc.cpp

@@ -1990,8 +1990,11 @@ void EclCC::outputXmlToOutputFile(EclCompileInstance & instance, IPropertyTree *
     {
         //Work around windows problem writing 64K to stdout if not redirected/piped
         Owned<IIOStream> stream = createIOStream(ifileio.get());
-        Owned<IIOStream> buffered = createBufferedIOStream(stream,0x8000);
-        saveXML(*buffered, xml);
+        {
+            Owned<IIOStream> buffered = createBufferedIOStream(stream,0x8000);
+            saveXML(*buffered, xml);
+        }
+        ifileio->close();
     }
 }
 
@@ -2044,7 +2047,10 @@ void EclCC::generateOutput(EclCompileInstance & instance)
 
             OwnedIFileIO ifileio = createArchiveOutputFile(instance);
             if (ifileio)
+            {
                 ifileio->write(0, filenames.length(), filenames.str());
+                ifileio->close();
+            }
         }
         else
         {

ecl/eclcmd/queries/ecl-queries.cpp

@@ -1382,7 +1382,10 @@ class EclCmdQueriesExport : public EclCmdCommon
         fprintf(stdout, "\nWriting to file %s\n", file->queryFilename());
 
         if (io.get())
+        {
             io->write(0, strlen(s), s);
+            io->close();
+        }
         else
             fprintf(stderr, "\nFailed to create file %s\n", file->queryFilename());
     }

ecl/hql/hqlcache.cpp

@@ -492,6 +492,7 @@ static void extractFile(const char * path, const char * moduleName, const char *
     Owned<IFileIO> io = file->open(IFOcreate);
     if (text)
         io->write(0, strlen(text), text);
+    io->close();
     io.clear();
     if (ts)
     {

ecl/hqlcpp/hqlecl.cpp

@@ -310,7 +310,7 @@ void HqlDllGenerator::expandCode(StringBuffer & filename, const char * codeTempl
     addDirectoryPrefix(fullname, targetDir).append(filename);
 
     Owned<IFile> out = createIFile(fullname.str());
-    Owned<ITemplateExpander> expander = createTemplateExpander(out, codeTemplate);
+    Owned<ITemplateExpander> expander = createTemplateExpander(codeTemplate);
 
     Owned<ISectionWriter> writer = createCppWriter(*code, compiler);
     Owned<IProperties> props = createProperties(true);
@@ -324,7 +324,7 @@ void HqlDllGenerator::expandCode(StringBuffer & filename, const char * codeTempl
     props->setProp("headerName", headerName.str());
     props->setProp("outputName", fullname.str());
 
-    expander->generate(*writer, pass, props);
+    expander->generate(*writer, out, pass, props);
 
     totalGeneratedSize += out->size();
 

ecl/hqlcpp/hqlwcpp.cpp

@@ -103,9 +103,14 @@ CppWriterTemplate::CppWriterTemplate(const char * codeTemplate)
     loadTemplate(codeTemplate);
 }
 
-void CppWriterTemplate::generate(ISectionWriter & writer, unsigned pass, IProperties * properties)
+void CppWriterTemplate::generate(ISectionWriter & writer, IFile * outputFile, unsigned pass, IProperties * properties)
 {
-    writer.setOutput(out, outStream);
+    Owned<IFileIO> io = outputFile->open(IFOcreate);
+    if (!io)
+        throwError1(HQLERR_CouldNotCreateOutputX, outputFile->queryFilename());
+
+    outStream.setown(createIOStream(io));
+    writer.setOutput(outputFile, outStream);
 
     const char * finger = text;
     bool output = true;
@@ -149,6 +154,8 @@ void CppWriterTemplate::generate(ISectionWriter & writer, unsigned pass, IProper
         outputQuoted(writer, end-finger, finger);
 
     writer.setOutput(NULL, NULL);
+    outStream.clear();
+    io->close();
 }
 
 void CppWriterTemplate::loadTemplate(const char * codeTemplate)
@@ -2271,11 +2278,9 @@ void HqlCppSectionWriter::generateSection(unsigned delta, IAtom * section, unsig
 
 //---------------------------------------------------------------------------
 
-ITemplateExpander * createTemplateExpander(IFile * output, const char * codeTemplate)
+ITemplateExpander * createTemplateExpander(const char * codeTemplate)
 {
-    Owned<CppWriterTemplate> expander = new CppWriterTemplate(codeTemplate);
-    expander->setOutput(output);
-    return expander.getClear();
+    return new CppWriterTemplate(codeTemplate);
 }
 
 ISectionWriter * createCppWriter(IHqlCppInstance & _instance, CompilerType compiler)

ecl/hqlcpp/hqlwcpp.hpp

@@ -35,15 +35,15 @@ interface HQLCPP_API ISectionWriter : public IInterface
 interface HQLCPP_API ITemplateExpander : public IInterface
 {
 public:
-    virtual void generate(ISectionWriter & writer, unsigned pass, IProperties * properties = NULL) = 0;
+    virtual void generate(ISectionWriter & writer, IFile * _output, unsigned pass, IProperties * properties = NULL) = 0;
 };
 
 extern HQLCPP_API StringBuffer & generateExprCpp(StringBuffer & out, IHqlExpression * expr, CompilerType compiler);
 extern HQLCPP_API StringBuffer & generateTypeCpp(StringBuffer & out, ITypeInfo * type, const char * name, CompilerType compiler);
 bool generateFunctionPrototype(StringBuffer & out, IHqlExpression * funcdef, CompilerType compiler);
 void generateFunctionReturnType(StringBuffer & prefix, StringBuffer & params, ITypeInfo * retType, IHqlExpression * attrs, CompilerType compiler);
 
-extern HQLCPP_API ITemplateExpander * createTemplateExpander(IFile * output, const char * codeTemplate);
+extern HQLCPP_API ITemplateExpander * createTemplateExpander(const char * codeTemplate);
 extern HQLCPP_API ISectionWriter * createCppWriter(IHqlCppInstance & _instance, CompilerType compiler);
 extern bool isTypePassedByAddress(ITypeInfo * type);
 

ecl/hqlcpp/hqlwcpp.ipp

@@ -23,25 +23,14 @@ public:
     CppWriterTemplate(const char * codeTemplate);
     IMPLEMENT_IINTERFACE
 
-    virtual void generate(ISectionWriter & writer, unsigned pass, IProperties * properties = NULL);
+    virtual void generate(ISectionWriter & writer, IFile * _output, unsigned pass, IProperties * properties = NULL);
 
     void outputQuoted(ISectionWriter & writer, size32_t len, const char * str)
     {
         writer.noteLines(memcount(len, str, '\n'));
         outStream->write(len, str);
     }
 
-    void setOutput(IFile * _output)
-    {
-        Owned<IFileIO> io = _output->open(IFOcreate);
-        if (!io)
-            throwError1(HQLERR_CouldNotCreateOutputX, _output->queryFilename());
-
-        out.set(_output);
-        outStream.setown(createIOStream(io));
-    }
-
-
 private:
     void loadTemplate(const char * codeTemplate);
     enum TplSectionType { TplEmbed, TplExpand, TplCondition, TplEndCondition };
@@ -58,7 +47,6 @@ protected:
     const char * text;
     unsigned len;
     CIArray sections;
-    Owned<IFile> out;
     Owned<IIOStream> outStream;
 };
 

ecl/hthor/hthor.cpp

@@ -1295,6 +1295,7 @@ void CHThorIndexWriteActivity::execute()
         offsetBranches = builder->getOffsetBranches();
         out->flush();
         out.clear();
+        io->close();
     }
 
     if(clusterHandler)

system/jlib/jcomp.cpp

@@ -475,7 +475,10 @@ void CppCompiler::writeLogFile(const char* filepath, StringBuffer& log)
 
     Owned <IFileIO> fio = f->open(IFOcreate);
     if(fio.get())
+    {
         fio->write(0, log.length(), log.str());
+        fio->close();
+    }
 }
 
 bool CppCompiler::compile()
@@ -560,6 +563,7 @@ bool CppCompiler::compile()
 
         //Don't leave lots of blank log files around if the compile was successful
         bool logIsEmpty = (dstIO->size() == 0);
+        dstIO->close();
         dstIO.clear();
         if (ret && logIsEmpty)
             dstfile->remove();

system/jlib/jfile.cpp

@@ -73,6 +73,9 @@
                                             // this should not be enabled in WindowRemoteDirectory used
 //#define CHECK_FILE_IO           // If enabled, reads and writes are checked for sensible parameters
 
+#ifdef _DEBUG
+//#define CHECK_FILE_CLOSED_BEFORE_DELETE
+#endif
 
 #ifdef _DEBUG
 #define ASSERTEX(e) assertex(e); 
@@ -2075,13 +2078,22 @@ CFileIO::CFileIO(HANDLE handle, IFOmode _openmode, IFSHmode _sharemode, IFEflags
 
 CFileIO::~CFileIO()
 {
+#ifdef CHECK_FILE_CLOSED_BEFORE_DELETE
+    //Any file that is being written to, should be closed before the object is destroyed, otherwise errors from failing to commit will be lost
+    if ((file != NULLFILE) && (openmode!=IFOread))
+    {
+        OERRLOG("CFileIO::~CFileIO - file object destroyed without being closed first"); // A programmer problem, but the operator should know about it.
+        PrintStackReport();
+    }
+#endif
     try
     {
         close();
     }
     catch (IException * e)
     {
-        EXCLOG(e, "CFileIO::~CFileIO");
+        //An error closing a file cannot throw an exception, but should be logged as a very severe error in the logs.
+        DISLOG(e, "CFileIO::~CFileIO");
         PrintStackReport();
         e->Release();
     }
@@ -3289,6 +3301,7 @@ void doCopyFile(IFile * target, IFile * source, size32_t buffersize, ICopyFilePr
             if (progress && progress->onProgress(offset, total) != CFPcontinue)
                 break;
         }
+        targetIO->close(); // Ensure errors are reported.
         targetIO.clear();
         if (usetmp) {
             StringAttr tail(pathTail(target->queryFilename()));
@@ -4441,6 +4454,7 @@ void touchFile(IFile *iFile)
     Owned<IFileIO> iFileIO = iFile->open(IFOcreate);
     if (!iFileIO)
         throw makeStringExceptionV(0, "touchFile: failed to create file %s", iFile->queryFilename());
+    iFileIO->close();
 }
 
 void touchFile(const char *filename)
@@ -7323,6 +7337,7 @@ extern jlib_decl void writeSentinelFile(IFile * sentinelFile)
         {
             Owned<IFileIO> sentinel = sentinelFile->open(IFOcreate);
             sentinel->write(0, 5, "rerun");
+            sentinel->close();
         }
         catch(IException *E)
         {

system/jlib/jlzw.cpp

@@ -2077,39 +2077,51 @@ class CCompressedFile : implements ICompressedFileIO, public CInterface
 
         //If the blocks are being expanded incrementally check if the position is within the current block
         //This test will never be true for row compressed data, or non-incremental decompression
-        if ((pos >= startBlockPos) && (pos < startBlockPos + fullBlockSize))
+        try
         {
-            if (pos < nextExpansionPos)
+            if ((pos >= startBlockPos) && (pos < startBlockPos + fullBlockSize))
             {
-                //Start decompressing again and avoid re-reading the data from disk
-                const void * rawData;
-                if (fileio)
-                    rawData = compressedInputBlock.get();
-                else
-                    rawData = mmfile->base()+startBlockPos;
-
-                assertex(rawData);
-                size32_t exp = expander->expandFirst(curblockbuf, rawData);
-                curblockpos = startBlockPos;
-                nextExpansionPos = startBlockPos + exp;
                 if (pos < nextExpansionPos)
-                    return;
+                {
+                    //Start decompressing again and avoid re-reading the data from disk
+                    const void * rawData;
+                    if (fileio)
+                        rawData = compressedInputBlock.get();
+                    else
+                        rawData = mmfile->base()+startBlockPos;
+
+                    assertex(rawData);
+                    nextExpansionPos = startBlockPos; // update in case an exception is thrown
+                    size32_t exp = expander->expandFirst(curblockbuf, rawData);
+                    curblockpos = startBlockPos;
+                    nextExpansionPos = startBlockPos + exp;
+                    if (pos < nextExpansionPos)
+                        return;
 
-                curblockbuf.clear();
-            }
+                    curblockbuf.clear();
+                }
 
-            for (;;)
-            {
-                size32_t nextSize = expander->expandNext(curblockbuf);
-                if (nextSize == 0)
-                    throw makeStringExceptionV(-1, "Unexpected zero length compression block at position %llu", curblockpos);
+                for (;;)
+                {
+                    size32_t nextSize = expander->expandNext(curblockbuf);
+                    if (nextSize == 0)
+                        throw makeStringException(-1, "Unexpected zero length compression block");
 
-                curblockpos = nextExpansionPos;
-                nextExpansionPos = nextExpansionPos+nextSize;
-                if (pos < nextExpansionPos)
-                    return;
+                    curblockpos = nextExpansionPos;
+                    nextExpansionPos = nextExpansionPos+nextSize;
+                    if (pos < nextExpansionPos)
+                        return;
+                }
             }
         }
+        catch (IException * e)
+        {
+            unsigned code = e->errorCode();
+            StringBuffer msg;
+            e->errorMessage(msg).appendf(" at position %llu of %llu", nextExpansionPos, trailer.indexPos);
+            e->Release();
+            throw makeStringException(code, msg.str());
+        }
 
         size32_t expsize;
         curblocknum = lookupIndex(pos,curblockpos,expsize);
@@ -2456,6 +2468,8 @@ class CCompressedFile : implements ICompressedFileIO, public CInterface
             }
             checkedwrite(trailer.indexPos,indexbuf.length(),indexbuf.toByteArray());
             indexbuf.clear();
+            if (fileio)
+                fileio->close();
         }
         mode = ICFread;
         curblockpos = 0;

system/jlib/jptree.cpp

@@ -6332,6 +6332,7 @@ void saveXML(IFile &ifile, const IPropertyTree *tree, unsigned indent, unsigned
     if (!ifileio)
         throw MakeStringException(0, "saveXML: could not find %s to open", ifile.queryFilename());
     saveXML(*ifileio, tree, indent, flags);
+    ifileio->close(); // Ensure errors are reported
 }
 
 void saveXML(IFileIO &ifileio, const IPropertyTree *tree, unsigned indent, unsigned flags)
@@ -9692,6 +9693,7 @@ void saveYAML(IFile &ifile, const IPropertyTree *tree, unsigned indent, unsigned
     if (!ifileio)
         throw MakeStringException(0, "saveXML: could not find %s to open", ifile.queryFilename());
     saveYAML(*ifileio, tree, indent, flags);
+    ifileio->close();
 }
 
 void saveYAML(IFileIO &ifileio, const IPropertyTree *tree, unsigned indent, unsigned flags)