HPCC-25380 Add a class for catching incorrect use of IFileIO

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

common/remote/hooks/azure/azurefile.cpp

@@ -182,10 +182,8 @@ public:
     }
     virtual IFileIO * open(IFOmode mode, IFEflags extraFlags=IFEnone) override
     {
-        if (mode == IFOcreate)
-            return createFileWriteIO();
-        assertex(mode==IFOread);
-        return createFileReadIO();
+        //Should this be derived from a comman base that implements the setShareMode()?
+        return openShared(mode,IFSHread,extraFlags);
     }
     virtual IFileAsyncIO * openAsync(IFOmode mode)
     {

system/jlib/jfile.cpp

@@ -65,6 +65,8 @@
 
 // #define REMOTE_DISCONNECT_ON_DESTRUCTOR  // enable to disconnect on IFile destructor
                                             // 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 ASSERTEX(e) assertex(e); 
@@ -1731,7 +1733,13 @@ IFileIO * CFile::openShared(IFOmode mode,IFSHmode share,IFEflags extraFlags)
 #endif
     if (stdh>=0)
         return new CSequentialFileIO(handle,mode,share,extraFlags);
-    return new CFileIO(handle,mode,share,extraFlags);
+
+    Owned<IFileIO> io = new CFileIO(handle,mode,share,extraFlags);
+#ifdef CHECK_FILE_IO
+    return new CCheckingFileIO(filename, io);
+#else
+    return io.getClear();
+#endif
 }
 
 
@@ -2087,6 +2095,86 @@ size32_t CFileRangeIO::write(offset_t pos, size32_t len, const void * data)
 
 //--------------------------------------------------------------------------
 
+CCheckingFileIO::~CCheckingFileIO()
+{
+    if (lastWriteSize && !closed)
+        report("FileCheck: IO for File %s destroyed before closing", filename.str());
+}
+
+size32_t CCheckingFileIO::read(offset_t pos, size32_t len, void * data)
+{
+    CriticalBlock block(cs);
+    if ((pos == lastReadPos) && (len < minSeqReadSize) && (pos + len != size()))
+        report("FileCheck: Sequential read [%s] of %u is < %u", filename.str(), len, minSeqReadSize);
+
+    size32_t numRead = io->read(pos, len, data);
+    lastReadPos = pos + numRead;
+    return numRead;
+}
+
+offset_t CCheckingFileIO::size()
+{
+    return io->size();
+}
+
+size32_t CCheckingFileIO::write(offset_t pos, size32_t len, const void * data)
+{
+    CriticalBlock block(cs);
+    if (len != 0)
+    {
+        if ((lastWriteSize != 0) && (lastWriteSize < minWriteSize))
+            report("FileCheck: Sequential write to [%s] of size %u before offset %" I64F "u of %u is < %u", filename.str(), lastWriteSize, pos, len, minWriteSize);
+        lastWriteSize = len;
+    }
+    else
+        report("FileCheck: Unexpected zero byte write on %s at offset %" I64F "u", filename.str(), pos);
+
+    return io->write(pos, len, data);
+}
+
+void CCheckingFileIO::setSize(offset_t size)
+{
+    io->setSize(size);
+}
+
+offset_t CCheckingFileIO::appendFile(IFile *file,offset_t pos,offset_t len)
+{
+    return io->appendFile(file, pos, len);
+}
+
+void CCheckingFileIO::flush()
+{
+    io->flush();
+}
+
+void CCheckingFileIO::close()
+{
+    io->close();
+    closed = true;
+}
+
+unsigned __int64 CCheckingFileIO::getStatistic(StatisticKind kind)
+{
+    return io->getStatistic(kind);
+}
+
+void CCheckingFileIO::report(const char * format, ...)
+{
+    va_list args;
+    va_start(args, format);
+    VALOG(MCinternalError, unknownJob, format, args);
+    va_end(args);
+    if (!traced)
+    {
+        printStackReport();
+        traced = true;
+    }
+}
+
+
+
+//--------------------------------------------------------------------------
+
 CFileAsyncIO::~CFileAsyncIO()
 {
     try

system/jlib/jfile.ipp

@@ -104,12 +104,8 @@ public:
     virtual void close();
     virtual unsigned __int64 getStatistic(StatisticKind kind);
 
-    bool create(const char * filename, bool replace);
-    bool open(const char * filename);
-
     HANDLE queryHandle() { return file; } // for debugging
 
-
 protected:
     CriticalSection     cs;
     HANDLE              file;
@@ -146,6 +142,38 @@ protected:
     offset_t            maxLength;
 };
 
+//A wrapper class than can be used to ensure the interface is used in a sensible way - e.g.
+//files are closed before destruction when writing.  Sensible buffering is in place.
+class jlib_decl CCheckingFileIO : implements CInterfaceOf<IFileIO>
+{
+public:
+    CCheckingFileIO(const char * _filename, IFileIO * _io) : filename(_filename), io(_io) {}
+    ~CCheckingFileIO();
+
+    virtual size32_t read(offset_t pos, size32_t len, void * data) override;
+    virtual offset_t size() override;
+    virtual size32_t write(offset_t pos, size32_t len, const void * data) override;
+    virtual void setSize(offset_t size) override;
+    virtual offset_t appendFile(IFile *file,offset_t pos,offset_t len) override;
+    virtual void flush() override;
+    virtual void close() override;
+    virtual unsigned __int64 getStatistic(StatisticKind kind) override;
+
+protected:
+    void report(const char * format, ...) __attribute__((format(printf, 2, 3)));
+
+protected:
+    CriticalSection cs;
+    StringAttr filename;
+    Linked<IFileIO> io;
+    bool closed = false;
+    bool traced = false;
+    unsigned minSeqReadSize = 0x10000;
+    unsigned minWriteSize = 0x010000;
+    offset_t lastReadPos = (offset_t)-1;
+    size32_t lastWriteSize = 0;
+};
+
 
 class jlib_decl CFileAsyncIO : implements IFileAsyncIO, public CInterface
 {

system/jlib/jptree.cpp

@@ -4548,7 +4548,7 @@ public:
     CommonReaderBase(ISimpleReadStream &_stream, IPTreeNotifyEvent &_iEvent, PTreeReaderOptions _readerOptions, size32_t _bufSize=0) :
         bufSize(_bufSize), readerOptions(_readerOptions), iEvent(&_iEvent)
     {
-        if (!bufSize) bufSize = 0x8000;
+        if (!bufSize) bufSize = 0x20000;
         buf = new byte[bufSize];
         bufRemaining = 0;
         curOffset = 0;