Strong exception safety in all uses of XMLFileWriter...

... Strong, meaning that the file at the specified path is created or modified
only if all write operations complete without exceptions, barring one very
unlikely possibility that a final file rename fails, but even in that case the
output is successfully written to some path.

This commit does not add throws, but changes the type thrown to a subclass of
AudacityException, so that GuardedCall will cause the user to see an error
dialog in all cases.

Duplicated logic for making temporary files and backups is now all in one
place, the class XMLWriter.

There may be more new GuardedCalls than necessary -- the catch-all for the
event loop, AudacityApp::OnExceptionInMainLoop, might be trusted instead in
some cases --  but they are sufficient.

src/xml/XMLWriter.cpp

@@ -265,58 +265,107 @@ wxString XMLWriter::XMLEsc(const wxString & s)
 ///
 /// XMLFileWriter class
 ///
-XMLFileWriter::XMLFileWriter()
+XMLFileWriter::XMLFileWriter
+   ( const wxString &outputPath, const wxString &caption, bool keepBackup )
+   : mOutputPath{ outputPath }
+   , mCaption{ caption }
+   , mKeepBackup{ keepBackup }
+// may throw
 {
-}
+   auto tempPath = wxFileName::CreateTempFileName( outputPath );
+   if (!wxFFile::Open(tempPath, wxT("wb")) || !IsOpened())
+      ThrowException( tempPath, mCaption );
 
-XMLFileWriter::~XMLFileWriter()
-{
-   if (IsOpened()) {
-      Close();
+   if (mKeepBackup) {
+      int index = 0;
+      wxString backupName;
+
+      do {
+         wxFileName outputFn{ mOutputPath };
+         index++;
+         mBackupName =
+         outputFn.GetPath() + wxFILE_SEP_PATH +
+         outputFn.GetName() + wxT("_bak") +
+         wxString::Format(wxT("%d"), index) + wxT(".") +
+         outputFn.GetExt();
+      } while( ::wxFileExists( mBackupName ) );
+
+      // Open the backup file to be sure we can write it and reserve it
+      // until committing
+      if (! mBackupFile.Open( mBackupName, "wb" ) || ! mBackupFile.IsOpened() )
+         ThrowException( mBackupName, mCaption );
    }
 }
 
-void XMLFileWriter::Open(const wxString &name, const wxString &mode)
+
+XMLFileWriter::~XMLFileWriter()
 {
-   if (!wxFFile::Open(name, mode))
-      throw XMLFileWriterException(_("Error Opening File"));
+   // Don't let a destructor throw!
+   GuardedCall< void >( [&] {
+      if (IsOpened()) {
+         // Was not committed
+         auto fileName = GetName();
+         CloseWithoutEndingTags();
+         ::wxRemoveFile( fileName );
+      }
+   } );
 }
 
-void XMLFileWriter::Close()
+void XMLFileWriter::Commit()
 // may throw
 {
    while (mTagstack.GetCount()) {
       EndTag(mTagstack[0]);
    }
 
+   auto tempPath = GetName();
    CloseWithoutEndingTags();
+
+   if (mKeepBackup) {
+      if (! mBackupFile.Close() ||
+          ! wxRenameFile( mOutputPath, mBackupName ) )
+         ThrowException( mBackupName, mCaption );
+   }
+   else {
+      if ( ! wxRemoveFile( mOutputPath ) )
+         ThrowException( mOutputPath, mCaption );
+   }
+
+   // Now we have vacated the file at the output path and are committed.
+   // But not completely finished with steps of the commit operation.
+   // If this step fails, we haven't lost the successfully written data,
+   // but just failed to put it in the right place.
+   if (! wxRenameFile( tempPath, mOutputPath ) )
+      throw FileException{
+         FileException::Cause::Rename, tempPath, mCaption, mOutputPath
+      };
 }
 
 void XMLFileWriter::CloseWithoutEndingTags()
+// may throw
 {
    // Before closing, we first flush it, because if Flush() fails because of a
    // "disk full" condition, we can still at least try to close the file.
    if (!wxFFile::Flush())
    {
       wxFFile::Close();
-      /* i18n-hint: 'flushing' means writing any remaining queued up changes
-       * to disk that have not yet been written.*/
-      throw XMLFileWriterException(_("Error Flushing File"));
+      ThrowException( GetName(), mCaption );
    }
 
    // Note that this should never fail if flushing worked.
    if (!wxFFile::Close())
-      throw XMLFileWriterException(_("Error Closing File"));
+      ThrowException( GetName(), mCaption );
 }
 
 void XMLFileWriter::Write(const wxString &data)
+// may throw
 {
-   if (!wxFFile::Write(data, wxConvUTF8))
+   if (!wxFFile::Write(data, wxConvUTF8) || Error())
    {
       // When writing fails, we try to close the file before throwing the
       // exception, so it can at least be deleted.
       wxFFile::Close();
-      throw XMLFileWriterException(_("Error Writing to File"));
+      ThrowException( GetName(), mCaption );
    }
 }
 

src/xml/XMLWriter.h

@@ -14,6 +14,8 @@
 #include <wx/dynarray.h>
 #include <wx/ffile.h>
 
+#include "../FileException.h"
+
 ///
 /// XMLWriter
 ///
@@ -60,41 +62,55 @@ class AUDACITY_DLL_API XMLWriter /* not final */ {
 ///
 /// XMLFileWriter
 ///
-class AUDACITY_DLL_API XMLFileWriter final : public wxFFile, public XMLWriter {
+
+/// This writes to a provisional file, and replaces the previously existing
+/// contents by a file rename in Commit() only after all writes succeed.
+/// The original contents may also be retained at a backup path name, as
+/// directed by the optional constructor argument.
+/// If it is destroyed before Commit(), then the provisional file is removed.
+/// If the construction and all operations are inside a GuardedCall or event
+/// handler, then the default delayed handler action in case of exceptions will
+/// notify the user of problems.
+class AUDACITY_DLL_API XMLFileWriter final : private wxFFile, public XMLWriter {
 
  public:
 
-   XMLFileWriter();
+   /// The caption is for message boxes to show in case of errors.
+   /// Might throw.
+   XMLFileWriter
+      ( const wxString &outputPath, const wxString &caption,
+        bool keepBackup = false );
+
    virtual ~XMLFileWriter();
 
-   /// Open the file. Might throw XMLFileWriterException.
-   void Open(const wxString &name, const wxString &mode);
+   /// Close all tags and then close the file.
+   /// Might throw.  If not, then create
+   /// or modify the file at the output path.
+   void Commit();
 
-   /// Close file. Might throw XMLFileWriterException.
-   void Close();
-
-   /// Close file without automatically ending tags.
-   /// Might throw XMLFileWriterException.
-   void CloseWithoutEndingTags(); // for auto-save files
-
-   /// Write to file. Might throw XMLFileWriterException.
+   /// Write to file. Might throw.
    void Write(const wxString &data) override;
 
+   wxString GetBackupName() const { return mBackupName; }
+
  private:
 
-};
+   void ThrowException(
+      const wxFileName &fileName, const wxString &caption)
+   {
+      throw FileException{ FileException::Cause::Write, fileName, caption };
+   }
 
-///
-/// Exception thrown by various XMLFileWriter methods
-///
-class XMLFileWriterException
-{
-public:
-   XMLFileWriterException(const wxString& message) { mMessage = message; }
-   wxString GetMessage() const { return mMessage; }
+   /// Close file without automatically ending tags.
+   /// Might throw.
+   void CloseWithoutEndingTags(); // for auto-save files
 
-protected:
-   wxString mMessage;
+   const wxString mOutputPath;
+   const wxString mCaption;
+   wxString mBackupName;
+   const bool mKeepBackup;
+
+   wxFFile mBackupFile;
 };
 
 ///