FS: Explicit File::release() after reads/writes are done

The file system has been designed to not know how many files are currently in use, but NativeFile has an operating system specific limitation on the maximum number of open files.

Instead of trying to keep track of all open native files, ensure the handles are closed when file access is done. There were threading issues trying to flush open files from a thread while another thread is still accessing the file, or attempting to do its own flush.

File::release() releases all the internal buffers and resources, so it encompasses write flushing but also closing of the file handles after reading.

The idea here is for users to call File::release() after they're done with a file. Certain operations like Reader/Writer being destroyed will cause an automatic release.

doomsday/apps/client/src/audio/base/audiosystem.cpp

@@ -740,7 +740,7 @@ DE_PIMPL(AudioSystem)
                     Block buf(hndl->length());
                     hndl->read(buf.data(), buf.size());
                     file << buf;
-                    file.flush();
+                    file.release();
                     return iMusic->PlayFile(file.as<NativeFile>().nativePath(), looped);
                 }
                 else if (iMusic->Play && iMusic->SongBuffer)
@@ -790,7 +790,7 @@ DE_PIMPL(AudioSystem)
             Block buf(lump.size());
             lump.read(buf.data(), 0, lump.size());
             midi << M_Mus2Midi(buf);
-            midi.flush();
+            midi.release();
             //M_Free(buf); buf = nullptr;
 
             return forAllInterfaces(AUDIO_IMUSIC, [&midi, &looped] (void *ifs)

doomsday/apps/client/src/con_config.cpp

@@ -177,8 +177,6 @@ static bool writeConsoleState(const Path &filePath)
 
         out.writeText("\n#\n# ALIASES\n#\n\n");
         writeAliasesToFile(out);
-
-        file.flush();
     }
     catch (const Error &er)
     {
@@ -244,8 +242,6 @@ static bool writeBindingsState(const Path &filePath)
 
             return LoopContinue;
         });
-
-        file.flush();
         return true;
     }
     catch (const Error &er)

doomsday/apps/client/src/ui/widgets/gamewidget.cpp

@@ -257,7 +257,7 @@ void GameWidget::renderCubeMap(uint size, const String &outputImagePath)
 
         File &outFile = FS::get().root().replaceFile(uniquePath);
         outFile << buf;
-        outFile.flush();
+        outFile.release();
 
         LOG_GL_MSG("Cube map saved to \"%s\"") << outFile.correspondingNativePath();
     }

doomsday/libs/core/include/de/archiveentryfile.h

@@ -62,7 +62,7 @@ class DE_PUBLIC ArchiveEntryFile : public ByteArrayFile
      * manual flushing this occurs automatically when the root ArchiveFeed
      * instance is deleted.
      */
-    void flush() override;
+    void release() const override;
 
     Block metaId() const override;
 

doomsday/libs/core/include/de/archivefolder.h

@@ -51,7 +51,7 @@ class DE_PUBLIC ArchiveFolder : public Folder
 
     virtual ~ArchiveFolder();
 
-    void flush();
+    void release() const;
 
     String describe() const;
 

doomsday/libs/core/include/de/file.h

@@ -63,7 +63,7 @@ class Feed;
  * Subclasses have some special requirements for their destructors:
  * - deindex() must be called in all subclass destructors so that the instances indexed
  *   under the subclasses' type are removed from the file system's index, too.
- * - The file must be automatically flushed before it gets destroyed (see flush()).
+ * - The file must be automatically flushed before it gets destroyed (see release()).
  * - The deletion audience must be notified and @c audienceForDeletion must be cleared
  *   afterwards.
  *
@@ -148,14 +148,21 @@ class DE_PUBLIC File : public filesys::Node, public IIOStream, public IObject
      * Remove this file from its file system's index.
      */
     virtual void deindex();
-
+    
     /**
-     * Commits any buffered changes to the content of the file. All subclasses of File
-     * must make sure they flush themselves right before they get deleted. Subclasses
-     * must also flush themselves when a file in write mode is changed to read-only mode,
-     * if necessary.
+     * Commits any buffered changes to the content of the file and releases any internal
+     * buffers and temporary resources.
+     *
+     * All subclasses of File must make sure they release themselves right before they get
+     * deleted. Subclasses must also release themselves when a file in write mode is changed
+     * to read-only mode, if necessary.
+     *
+     * For example, native files need to close their native file handles. This should always
+     * be called after one is finished reading or writing a file, so resources are not
+     * needlessly kept around. However, if this is called too early, the file may need to
+     * reopen handles and reallocate buffers, degrading performance.
      */
-    virtual void flush();
+    virtual void release() const;
 
     /**
      * Empties the contents of the file.

doomsday/libs/core/include/de/nativefile.h

@@ -53,7 +53,7 @@ class DE_PUBLIC NativeFile : public ByteArrayFile
     Block metaId() const;
 
     void clear();
-    void flush();
+    void release() const;
 
     /**
      * Returns the native path of the file.

doomsday/libs/core/include/de/reader.h

@@ -39,6 +39,9 @@ class IIStream;
  * a IByteArray or IIStream interface). Byte order defaults to little-endian
  * but can be changed to big-endian.
  *
+ * When a Reader is destroyed, the source file is automatically released,
+ * if there is one.
+ *
  * Note about versioning: readers must be prepared to support old versions of
  * the serialization protocol in addition to the latest one for backwards
  * compatibility.

doomsday/libs/core/include/de/writer.h

@@ -40,6 +40,9 @@ class IOStream;
  * array object (anything with a IByteArray interface). Defaults to
  * little-endian byte order.
  *
+ * When a Writer is destroyed, the destination file is automatically released,
+ * if there is one.
+ *
  * Note about versioning: when instructed to include a header, Writer always
  * uses the latest version of serialization.
  *

doomsday/libs/core/src/core/filelogsink.cpp

@@ -41,7 +41,7 @@ void FileLogSink::flush()
 {
     if (_file)
     {
-        _file->flush();
+        _file->release();
     }
 }
 

doomsday/libs/core/src/data/bank.cpp

@@ -288,7 +288,7 @@ DE_PIMPL(Bank)
                 Writer(*serial).withHeader()
                         << source->modifiedAt()
                         << *data->asSerializable();
-                serial->flush();
+                serial->release();
             }
             catch (...)
             {

doomsday/libs/core/src/data/block.cpp

@@ -42,6 +42,11 @@ Block::Block(const IByteArray &other)
     other.get(0, data(), num);
     // Ensure it's null-terminated.
     data()[num] = 0;
+    if (const File *f = maybeAs<File>(other))
+    {
+        // We are done with this file.
+        f->release();
+    }
 }
 
 Block::Block(const Block &other)
@@ -126,6 +131,11 @@ void Block::copyFrom(const IByteArray &array, Offset at, Size count)
     // Read the other's data directly into our data buffer.
     resize(count);
     array.get(at, data(), count);
+    if (const File *f = maybeAs<File>(array))
+    {
+        // We are done with this file.
+        f->release();
+    }
 }
 
 void Block::resize(Size size)

doomsday/libs/core/src/data/profiles.cpp

@@ -273,7 +273,7 @@ void Profiles::serialize() const
     // Create the pack and update the file system.
     File &outFile = App::rootFolder().replaceFile(d->fileName());
     outFile << Block(os.str());
-    outFile.flush(); // we're done
+    outFile.release(); // we're done
 
     LOG_VERBOSE("Wrote \"%s\" with %i profile%s")
             << d->fileName() << count << (count != 1? "s" : "");

doomsday/libs/core/src/data/reader.cpp

@@ -22,6 +22,7 @@
 #include "de/block.h"
 #include "de/iserializable.h"
 #include "de/iistream.h"
+#include "de/file.h"
 #include "de/fixedbytearray.h"
 #include "de/byterefarray.h"
 #include "de/byteorder.h"
@@ -70,6 +71,14 @@ DE_PIMPL_NOREF(Reader)
         upgradeToByteArray();
     }
 
+    ~Impl()
+    {
+        if (const File *f = maybeAs<File>(source))
+        {
+            f->release();
+        }
+    }
+
     /**
      * Byte arrays provide more freedom with reading. If the streaming object
      * happens to support the byte array interface, Reader will use it instead.

doomsday/libs/core/src/data/refuge.cpp

@@ -82,7 +82,7 @@ void Refuge::write() const
         Writer(App::mutablePersistentData().entryBlock(d->persistentPath)).withHeader()
             << d->names;
 
-        App::persistPackFolder().flush();
+        App::persistPackFolder().release();
     }
 }
 

doomsday/libs/core/src/data/writer.cpp

@@ -61,6 +61,14 @@ DE_PIMPL_NOREF(Writer)
           offset     (other.offset     ),
           fixedOffset(other.fixedOffset)
     {}
+
+    ~Impl()
+    {
+        if (File *f = maybeAs<File>(destination))
+        {
+            f->release();
+        }
+    }
 
     void write(const IByteArray::Byte *ptr, dsize size)
     {

doomsday/libs/core/src/filesys/archiveentryfile.cpp

@@ -100,9 +100,9 @@ void ArchiveEntryFile::clear()
     setStatus(st);
 }
 
-void ArchiveEntryFile::flush()
+void ArchiveEntryFile::release() const
 {
-    ByteArrayFile::flush();
+    ByteArrayFile::release();
     if (ArchiveFeed *feed = maybeAs<ArchiveFeed>(originFeed()))
     {
         feed->rewriteFile();

doomsday/libs/core/src/filesys/archivefeed.cpp

@@ -131,7 +131,6 @@ DE_PIMPL(ArchiveFeed)
 
             file->clear();
             Writer(*file) << *arch;
-            file->flush();
         }
         else
         {

doomsday/libs/core/src/filesys/archivefolder.cpp

@@ -34,9 +34,9 @@ ArchiveFolder::~ArchiveFolder()
     deindex();
 }
 
-void ArchiveFolder::flush()
+void ArchiveFolder::release() const
 {
-    Folder::flush();
+    Folder::release();
     primaryFeed()->as<ArchiveFeed>().rewriteFile();
 }
 

doomsday/libs/core/src/filesys/file.cpp

@@ -75,7 +75,7 @@ File::~File()
 
     DE_NOTIFY(Deletion, i) i->fileBeingDeleted(*this);
 
-    flush();
+    release();
     if (d->source != this)
     {
         // If we own a source, get rid of it.
@@ -95,8 +95,14 @@ void File::deindex()
     fileSystem().deindex(*this);
 }
 
-void File::flush()
-{}
+void File::release() const
+{
+    const File *src = source();
+    if (src != this)
+    {
+        src->release();
+    }
+}
 
 void File::clear()
 {
@@ -283,7 +289,7 @@ void File::setMode(const Flags &newMode)
     // Implicitly flush the file before switching away from write mode.
     if (d->mode.testFlag(Write) && !newMode.testFlag(Write))
     {
-        flush();
+        release();
     }
 
     if (this != d->source)
@@ -349,7 +355,7 @@ File *File::reinterpret()
         deindex();
     }
 
-    original->flush();
+    original->release();
     File *result = fileSystem().interpret(original);
 
     // The interpreter should use whatever origin feed the file was previously using.

doomsday/libs/core/src/filesys/filesystem.cpp

@@ -314,7 +314,7 @@ File &FileSystem::copySerialized(const String &sourcePath, const String &destina
 
     File *dest = &fs.root().replaceFile(destinationPath);
     *dest << contents;
-    dest->flush();
+    dest->release();
 
     if (behavior & ReinterpretDestination)
     {