New wrapper for wxFileConfig to fix several bugs

Bug 2135 - Audacity crashes if launched with a locked pluginregistry.cfg file
Bug 2651 - If pluginregistry.cfg gets locked while Audacity is active the Plug-in Manager is blocked - with no help
Bug 2650 - Virgin launch fails with no pluginregistry.cfg file created
Bug 1264 - Writing to locked audacity.cfg not reported to user
Bug 2649 - Launching Audacity with a locked audacity.cfg file gives not one but three error messages - and no help
Bug 2652 - User is not warned if they launch Audacity with a locked pluginsettings.cfg file

src/AudacityApp.cpp

@@ -106,6 +106,7 @@ It handles initialization and termination by subclassing wxApp.
 #include "prefs/DirectoriesPrefs.h"
 #include "prefs/GUIPrefs.h"
 #include "tracks/ui/Scrubbing.h"
+#include "widgets/FileConfig.h"
 #include "widgets/FileHistory.h"
 
 #ifdef EXPERIMENTAL_EASY_CHANGE_KEY_BINDINGS
@@ -187,11 +188,11 @@ void PopulatePreferences()
    {
       const wxString fullPath{fn.GetFullPath()};
 
-      wxFileConfig ini(wxEmptyString,
+      FileConfig ini(wxEmptyString,
-                       wxEmptyString,
+                     wxEmptyString,
-                       fullPath,
+                     fullPath,
-                       wxEmptyString,
+                     wxEmptyString,
-                       wxCONFIG_USE_LOCAL_FILE);
+                     wxCONFIG_USE_LOCAL_FILE);
 
       wxString lang;
       if (ini.Read(wxT("/FromInno/Language"), &lang))
@@ -1247,14 +1248,6 @@ bool AudacityApp::OnInit()
    wxFileName configFileName(FileNames::DataDir(), wxT("audacity.cfg"));
    InitPreferences( configFileName );
    PopulatePreferences();
-   // This test must follow PopulatePreferences, because if an error message
-   // must be shown, we need internationalization to have been initialized
-   // first, which was done in PopulatePreferences
-   if ( !CheckWritablePreferences() ) {
-      ::AudacityMessageBox(
-         UnwritablePreferencesErrorMessage( configFileName ) );
-      return false;
-   }
 
 #if defined(__WXMSW__) && !defined(__WXUNIVERSAL__) && !defined(__CYGWIN__)
    this->AssociateFileTypes();

src/CMakeLists.txt

@@ -892,6 +892,8 @@ list( APPEND SOURCES
       widgets/ErrorDialog.h
       widgets/ExpandingToolBar.cpp
       widgets/ExpandingToolBar.h
+      widgets/FileConfig.cpp
+      widgets/FileConfig.h
       widgets/FileDialog/FileDialog.cpp
       widgets/FileDialog/FileDialog.h
       $<$<BOOL:${wxIS_WIN}>:

src/PluginManager.cpp

@@ -1929,7 +1929,7 @@ bool PluginManager::DropFile(const wxString &fileName)
 void PluginManager::Load()
 {
    // Create/Open the registry
-   wxFileConfig registry(wxEmptyString, wxEmptyString, FileNames::PluginRegistry());
+   FileConfig registry(wxEmptyString, wxEmptyString, FileNames::PluginRegistry());
 
    // If this group doesn't exist then we have something that's not a registry.
    // We should probably warn the user, but it's pretty unlikely that this will happen.
@@ -2008,7 +2008,7 @@ void PluginManager::Load()
    return;
 }
 
-void PluginManager::LoadGroup(wxFileConfig *pRegistry, PluginType type)
+void PluginManager::LoadGroup(FileConfig *pRegistry, PluginType type)
 {
 #ifdef __WXMAC__
    // Bug 1590: On Mac, we should purge the registry of Nyquist plug-ins
@@ -2269,15 +2269,8 @@ void PluginManager::LoadGroup(wxFileConfig *pRegistry, PluginType type)
 
 void PluginManager::Save()
 {
-   if (!wxFileName(FileNames::PluginRegistry()).IsFileWritable()) {
-      AudacityMessageBox(
-         XO("The plugin registry file is not writable:\n\n%s\n\nSettings were not saved.")
-            .Format(FileNames::PluginRegistry()));
-      return;
-   }
-
    // Create/Open the registry
-   wxFileConfig registry(wxEmptyString, wxEmptyString, FileNames::PluginRegistry());
+   FileConfig registry(wxEmptyString, wxEmptyString, FileNames::PluginRegistry());
 
    // Clear it out
    registry.DeleteAll();
@@ -2303,7 +2296,7 @@ void PluginManager::Save()
    registry.Flush();
 }
 
-void PluginManager::SaveGroup(wxFileConfig *pRegistry, PluginType type)
+void PluginManager::SaveGroup(FileConfig *pRegistry, PluginType type)
 {
    wxString group = GetPluginTypeString(type);
    for (PluginMap::iterator iter = mPlugins.begin(); iter != mPlugins.end(); ++iter)
@@ -2780,11 +2773,11 @@ PluginDescriptor & PluginManager::CreatePlugin(const PluginID & id,
    return plug;
 }
 
-wxFileConfig *PluginManager::GetSettings()
+FileConfig *PluginManager::GetSettings()
 {
    if (!mSettings)
    {
-      mSettings = std::make_unique<wxFileConfig>(wxEmptyString, wxEmptyString, FileNames::PluginSettings());
+      mSettings = std::make_unique<FileConfig>(wxEmptyString, wxEmptyString, FileNames::PluginSettings());
 
       // Check for a settings version that we can understand
       if (mSettings->HasEntry(SETVERKEY))
@@ -2812,7 +2805,7 @@ wxFileConfig *PluginManager::GetSettings()
 
 bool PluginManager::HasGroup(const RegistryPath & group)
 {
-   wxFileConfig *settings = GetSettings();
+   FileConfig *settings = GetSettings();
 
    bool res = settings->HasGroup(group);
    if (res)

src/PluginManager.h

@@ -21,6 +21,7 @@
 #include "audacity/PluginInterface.h"
 
 class wxArrayString;
+class FileConfig;
 
 ///////////////////////////////////////////////////////////////////////////////
 //
@@ -270,13 +271,13 @@ private:
    ~PluginManager();
 
    void Load();
-   void LoadGroup(wxFileConfig *pRegistry, PluginType type);
+   void LoadGroup(FileConfig *pRegistry, PluginType type);
    void Save();
-   void SaveGroup(wxFileConfig *pRegistry, PluginType type);
+   void SaveGroup(FileConfig *pRegistry, PluginType type);
 
    PluginDescriptor & CreatePlugin(const PluginID & id, ComponentInterface *ident, PluginType type);
 
-   wxFileConfig *GetSettings();
+   FileConfig *GetSettings();
 
    bool HasGroup(const RegistryPath & group);
    bool GetSubgroups(const RegistryPath & group, RegistryPaths & subgroups);
@@ -312,7 +313,7 @@ private:
 
    bool IsDirty();
    void SetDirty(bool dirty = true);
-   std::unique_ptr<wxFileConfig> mSettings;
+   std::unique_ptr<FileConfig> mSettings;
 
    bool mDirty;
    int mCurrentIndex;

src/Prefs.cpp

@@ -178,12 +178,12 @@ AudacityPrefs::AudacityPrefs(const wxString& appName,
                const wxString& globalFilename,
                long style,
                const wxMBConv& conv) :
-   wxFileConfig(appName,
+   FileConfig(appName,
-               vendorName,
+              vendorName,
-               localFilename,
+              localFilename,
-               globalFilename,
+              globalFilename,
-               style,
+              style,
-               conv)
+              conv)
 {
 }
 
@@ -202,18 +202,6 @@ void InitPreferences( const wxFileName &configFileName )
    wxConfigBase::Set(gPrefs);
 }
 
-bool CheckWritablePreferences()
-{
-   return gPrefs->Write("/TEST", true) && gPrefs->Flush();
-}
-
-TranslatableString UnwritablePreferencesErrorMessage( const wxFileName &configFileName )
-{
-   return
-     XO("Audacity cannot start because the settings file at %s is not writable.")
-        .Format(configFileName.GetFullPath());
-}
-
 void FinishPreferences()
 {
    if (gPrefs) {

src/Prefs.h

@@ -33,16 +33,14 @@
 
 #include "../include/audacity/ComponentInterface.h"
 #include "MemoryX.h" // for wxArrayStringEx
+#include "widgets/FileConfig.h"
 
 #include <memory>
-#include <wx/fileconf.h>  // to inherit wxFileConfig
 #include <wx/event.h> // to declare custom event types
 
 class wxFileName;
 
 void InitPreferences( const wxFileName &configFileName );
-bool CheckWritablePreferences();
-TranslatableString UnwritablePreferencesErrorMessage( const wxFileName &configFileName );
 void FinishPreferences();
 
 class AudacityPrefs;
@@ -55,7 +53,7 @@ extern int gMenusDirty;
 /// \brief Our own specialisation of wxFileConfig.  It is essentially a renaming,
 /// though it does provide one new access function.  Most of the prefs work
 /// is actually done by the InitPreferences() function.
-class  AUDACITY_DLL_API AudacityPrefs : public wxFileConfig 
+class  AUDACITY_DLL_API AudacityPrefs : public FileConfig
 {
 public:
    AudacityPrefs(const wxString& appName = {},

src/widgets/FileConfig.cpp

@@ -0,0 +1,274 @@
+/**********************************************************************
+
+  Audacity: A Digital Audio Editor
+
+  FileConfig.cpp
+
+  Leland Lucius
+
+**********************************************************************/
+
+#include "../Audacity.h"
+
+#include <wx/defs.h>
+#include <wx/app.h>
+#include <wx/bmpbuttn.h>
+#include <wx/button.h>
+#include <wx/filefn.h>
+#include <wx/fileconf.h>
+#include <wx/sizer.h>
+#include <wx/wfstream.h>
+
+#include "FileConfig.h"
+#include "HelpSystem.h"
+#include "wxPanelWrapper.h"
+#include "../ShuttleGui.h"
+
+#include "../../images/Help.xpm"
+
+#if !defined(F_OK)
+#define F_OK 0x00
+#endif
+#if !defined(W_OK)
+#define W_OK 0x02
+#endif
+#if !defined(R_OK)
+#define R_OK 0x04
+#endif
+
+FileConfig::FileConfig(const wxString& appName,
+                       const wxString& vendorName,
+                       const wxString& localFilename,
+                       const wxString& globalFilename,
+                       long style,
+                       const wxMBConv& conv)
+:  wxFileConfig(appName, vendorName, localFilename, globalFilename, style, conv),
+   mConfigPath(localFilename),
+   mDirty(false)
+{
+   // Prevent wxFileConfig from attempting a Flush() during object deletion. This happens
+   // because we don't use the wxFileConfig::Flush() method and so the wxFileConfig dirty
+   // flag never gets reset. During deletiong it is checked a a Flush() performed. This
+   // can (and probably will) create bogus temporary files.
+   DisableAutoSave();
+
+   while (true)
+   {
+      bool canRead = false;
+      bool canWrite = false;
+      int fd;
+
+      fd = wxOpen(mConfigPath, O_RDONLY, S_IREAD);
+      if (fd != -1 || errno == ENOENT)
+      {
+         canRead = true;
+         if (fd != -1)
+         {
+            wxClose(fd);
+         }
+      }
+
+      fd = wxOpen(mConfigPath, O_WRONLY | O_CREAT, S_IWRITE);
+      if (fd != -1)
+      {
+         canWrite = true;
+         wxClose(fd);
+      }
+
+      if (canRead && canWrite)
+      {
+         break;
+      }
+
+      // If we can't read an existing config file, we must not allow the user to retry
+      // since the wxFileConfig initialization will not have read it and the caller
+      // will assume that the file was didn't exist and possibly initialize it. This
+      // could lead to wiping out the original contents.
+      //
+      // If the wxFileConfig class allowed us to call wxFileConfig::Init(), we wouldn't
+      // have to do all this mess.
+      Warn(canRead == true);
+   }
+}
+
+FileConfig::~FileConfig()
+{
+   wxASSERT(mDirty == false);
+}
+
+bool FileConfig::Flush(bool WXUNUSED(bCurrentOnly))
+{
+   if (!mDirty)
+   {
+      return true;
+   }
+
+   while (true)
+   {
+      FilePath backup = mConfigPath + ".bkp";
+
+      if (!wxFileExists(backup) || (wxRemove(backup) == 0))
+      {
+         if (!wxFileExists(mConfigPath) || (wxRename(mConfigPath, backup) == 0))
+         {
+            wxFileOutputStream stream(mConfigPath);
+            if (stream.IsOk())
+            {
+               if (Save(stream))
+               {
+                  stream.Sync();
+                  if (stream.IsOk() && stream.Close())
+                  {
+                     if (!wxFileExists(backup) || (wxRemove(backup) == 0))
+                     {
+                        mDirty = false;
+                        return true;
+                     }
+                  }
+               }
+            }
+
+            if (wxFileExists(backup))
+            {
+               wxRemove(mConfigPath);
+               wxRename(backup, mConfigPath);
+            }
+         }
+      }
+
+      Warn();
+   }
+
+   return false;
+}
+
+bool FileConfig::DoWriteString(const wxString& key, const wxString& szValue)
+{
+   bool res = wxFileConfig::DoWriteString(key, szValue);
+   if (res)
+   {
+      mDirty = true;
+   }
+   return res;
+}
+
+bool FileConfig::DoWriteLong(const wxString& key, long lValue)
+{
+   bool res = wxFileConfig::DoWriteLong(key, lValue);
+   if (res)
+   {
+      mDirty = true;
+   }
+   return res;
+}
+
+#if wxUSE_BASE64
+bool FileConfig::DoWriteBinary(const wxString& key, const wxMemoryBuffer& buf)
+{
+   bool res = wxFileConfig::DoWriteBinary(key, buf);
+   if (res)
+   {
+      mDirty = true;
+   }
+   return res;
+}
+#endif // wxUSE_BASE64
+
+void FileConfig::Warn(bool canRetry)
+{
+   wxDialogWrapper dlg(nullptr, wxID_ANY, XO("Audacity Configuration Error"));
+
+   ShuttleGui S(&dlg, eIsCreating);
+
+   wxButton *retryButton;
+   wxButton *quitButton;
+
+   S.SetBorder(5);
+   S.StartVerticalLay(wxEXPAND, 1);
+   {
+      S.SetBorder(15);
+      S.StartHorizontalLay(wxALIGN_RIGHT, 0);
+      {
+         auto cause = canRetry
+            ? XO("The following configuration file could not be written:")
+            : XO("The following configuration file could not be read:");
+            
+         auto retryMsg = canRetry
+            ? XO("You can attempt to correct the issue and then click \"Retry\" to coninue.\n\n")
+            : XO("");
+
+         S.AddFixedText(
+            XO("%s:\n\n"
+               "\t%s\n\n"
+               "This could be caused by many reasons, but the most likely are that "
+               "the disk is full or you do not have write permissions to the file. "
+               "More information can be obtained by clicking the help button below.\n\n"
+               "%s"
+               "If you choose to \"Quit Audacity\", your project may be left in an unsaved "
+               "state which will be recovered the next time you open it.")
+            .Format(cause, mConfigPath, retryMsg),
+            false,
+            500);
+      }
+      S.EndHorizontalLay();
+
+      S.SetBorder(5);
+      S.StartHorizontalLay(wxALIGN_RIGHT, 0);
+      {
+         // Can't use themed bitmap since the theme manager might not be
+         // initialized yet and it requires a configuration file.
+         wxButton *b = S.Id(wxID_HELP).AddBitmapButton(wxBitmap(Help_xpm));
+         b->SetToolTip( XO("Help").Translation() );
+         b->SetLabel(XO("Help").Translation());       // for screen readers
+
+         b = S.Id(wxID_CANCEL).AddButton(XXO("&Quit Audacity"));
+
+         if (canRetry)
+         {
+            b = S.Id(wxID_OK).AddButton(XXO("&Retry"));
+            dlg.SetAffirmativeId(wxID_OK);
+         }
+
+         b->SetDefault();
+         b->SetFocus();
+      }
+      S.EndHorizontalLay();
+   }
+   S.EndVerticalLay();
+
+   dlg.Layout();
+   dlg.GetSizer()->Fit(&dlg);
+   dlg.SetMinSize(dlg.GetSize());
+   dlg.Center();
+
+   auto onButton = [&](wxCommandEvent &e)
+   {
+      dlg.EndModal(e.GetId());
+   };
+
+   dlg.Bind(wxEVT_BUTTON, onButton);
+
+   switch (dlg.ShowModal())
+   {
+      case wxID_HELP:
+         // Can't use the HelpSystem since the theme manager may not
+         // yet be initialized and it requires a configuration file.
+         OpenInDefaultBrowser("https://" + 
+                              HelpSystem::HelpHostname +
+                              HelpSystem::HelpServerHomeDir +
+                              "Error:_Audacity_settings_file_unwritable");
+      break;
+
+      case wxID_CANCEL:
+         // This REALLY needs to use an exception with decent cleanup and program exit
+         wxExit();
+#if defined(__WXGTK__)
+         wxAbort();
+#else
+         exit(-1);
+#endif
+      break;
+   }
+
+   dlg.Unbind(wxEVT_BUTTON, onButton);
+}

src/widgets/FileConfig.h

@@ -0,0 +1,47 @@
+/**********************************************************************
+
+  Audacity: A Digital Audio Editor
+
+  FileConfig.h
+
+  Leland Lucius
+
+**********************************************************************/
+
+#ifndef __AUDACITY_WIDGETS_FILECONFIG__
+#define __AUDACITY_WIDGETS_FILECONFIG__
+
+#include <wx/defs.h>
+#include <wx/fileconf.h>
+
+#include "audacity/Types.h"
+
+class FileConfig : public wxFileConfig
+{
+public:
+   FileConfig(const wxString& appName = wxEmptyString,
+              const wxString& vendorName = wxEmptyString,
+              const wxString& localFilename = wxEmptyString,
+              const wxString& globalFilename = wxEmptyString,
+              long style = wxCONFIG_USE_LOCAL_FILE | wxCONFIG_USE_GLOBAL_FILE,
+              const wxMBConv& conv = wxConvAuto());
+   virtual ~FileConfig();
+
+   virtual bool Flush(bool bCurrentOnly = false) wxOVERRIDE;
+
+protected:
+   virtual bool DoWriteString(const wxString& key, const wxString& szValue) wxOVERRIDE;
+   virtual bool DoWriteLong(const wxString& key, long lValue) wxOVERRIDE;
+#if wxUSE_BASE64
+   virtual bool DoWriteBinary(const wxString& key, const wxMemoryBuffer& buf) wxOVERRIDE;
+#endif // wxUSE_BASE64
+
+private:
+   void Warn(bool canRetry = true);
+
+   FilePath mConfigPath;
+   bool mDirty;
+};
+
+#endif
+

src/widgets/FileHistory.cpp

@@ -17,7 +17,6 @@
 #include "FileHistory.h"
 
 #include <wx/defs.h>
-#include <wx/fileconf.h>
 #include <wx/menu.h>
 
 #include "../Internat.h"