Bug 2229 - Audacity may fail to load a "bad" module silently, with no error message

src/ModuleManager.cpp

@@ -97,48 +97,54 @@ Module::~Module()
 
 bool Module::Load()
 {
+   // Will this ever happen???
    if (mLib->IsLoaded()) {
       if (mDispatch) {
          return true;
       }
+
+      // Any messages should have already been generated the first time it was loaded.
       return false;
    }
 
-   if (!mLib->Load(mName, wxDL_LAZY)) {
+   auto ShortName = wxFileName(mName).GetName();
+
+   if (!mLib->Load(mName, wxDL_LAZY | wxDL_QUIET)) {
+      auto Error = wxSysErrorMsgStr();
+      AudacityMessageBox(XO("Unable to load the \"%s\" module.\n\nError: %s").Format(ShortName, Error),
+                         XO("Module Unsuitable"));
+      wxLogMessage(_("Unable to load the module \"%s\". Error: %s").Format(mName, Error));
       return false;
    }
 
    // Check version string matches.  (For now, they must match exactly)
    tVersionFn versionFn = (tVersionFn)(mLib->GetSymbol(wxT(versionFnName)));
    if (versionFn == NULL){
-      wxString ShortName = wxFileName( mName ).GetName();
       AudacityMessageBox(
-         XO(
-"The module %s does not provide a version string.\nIt will not be loaded.")
+         XO("The module \"%s\" does not provide a version string.\n\nIt will not be loaded.")
             .Format( ShortName),
          XO("Module Unsuitable"));
-      wxLogMessage(wxString::Format(_("The module %s does not provide a version string. It will not be loaded."), mName));
+      wxLogMessage(wxString::Format(_("The module \"%s\" does not provide a version string. It will not be loaded."), mName));
       mLib->Unload();
       return false;
    }
 
    wxString moduleVersion = versionFn();
    if( moduleVersion != AUDACITY_VERSION_STRING) {
-      wxString ShortName = wxFileName( mName ).GetName();
       AudacityMessageBox(
-         XO(
-"The module %s is matched with Audacity version %s.\n\nIt will not be loaded.")
-            .Format( ShortName, moduleVersion),
+         XO("The module \"%s\" is matched with Audacity version \"%s\".\n\nIt will not be loaded.")
+            .Format(ShortName, moduleVersion),
          XO("Module Unsuitable"));
-      wxLogMessage(wxString::Format(_("The module %s is matched with Audacity version %s. It will not be loaded."), mName, moduleVersion));
+      wxLogMessage(wxString::Format(_("The module \"%s\" is matched with Audacity version \"%s\". It will not be loaded."), mName, moduleVersion));
       mLib->Unload();
       return false;
    }
 
    mDispatch = (fnModuleDispatch) mLib->GetSymbol(wxT(ModuleDispatchName));
    if (!mDispatch) {
-      // Module does not provide a dispatch function...
-      // That can be OK, as long as we never try to call it.
+      // Module does not provide a dispatch function. Special case modules like this could be:
+      // (a) for scripting (RegScriptServerFunc entry point)
+      // (b) for hijacking the entire Audacity panel (MainPanelFunc entry point)
       return true;
    }
 
@@ -150,6 +156,13 @@ bool Module::Load()
    }
 
    mDispatch = NULL;
+
+   AudacityMessageBox(
+      XO("The module \"%s\" failed to initialize.\n\nIt will not be loaded.").Format(ShortName),
+      XO("Module Unsuitable"));
+   wxLogMessage(wxString::Format(_("The module \"%s\" failed to initialize.\nIt will not be loaded."), mName));
+   mLib->Unload();
+
    return false;
 }
 
@@ -305,22 +318,43 @@ void ModuleManager::Initialize(CommandHandler &cmdHandler)
       if (umodule->Load())   // it will get rejected if there  are version problems
       {
          auto module = umodule.get();
-         Get().mModules.push_back(std::move(umodule));
-         // We've loaded and initialised OK.
-         // So look for special case functions:
-         wxLogNull logNo; // Don't show wxWidgets errors if we can't do these. (Was: Fix bug 544.)
-         // (a) for scripting.
-         if( scriptFn == NULL )
-            scriptFn = (tpRegScriptServerFunc)(module->GetSymbol(wxT(scriptFnName)));
-         // (b) for hijacking the entire Audacity panel.
-         if( pPanelHijack==NULL )
+
          {
-            pPanelHijack = (tPanelFn)(module->GetSymbol(wxT(mainPanelFnName)));
+            // We've loaded and initialised OK.
+            // So look for special case functions:
+            wxLogNull logNo; // Don't show wxWidgets errors if we can't do these. (Was: Fix bug 544.)
+
+            // (a) for scripting.
+            if (scriptFn == NULL)
+            {
+               scriptFn = (tpRegScriptServerFunc)(module->GetSymbol(wxT(scriptFnName)));
+            }
+
+            // (b) for hijacking the entire Audacity panel.
+            if (pPanelHijack == NULL)
+            {
+               pPanelHijack = (tPanelFn)(module->GetSymbol(wxT(mainPanelFnName)));
+            }
          }
+
+         if (!module->HasDispatch() && !scriptFn && !pPanelHijack)
+         {
+            auto ShortName = wxFileName(files[i]).GetName();
+            AudacityMessageBox(
+               XO("The module \"%s\" does not provide any of the required functions.\n\nIt will not be loaded.").Format(ShortName),
+               XO("Module Unsuitable"));
+            wxLogMessage(wxString::Format(_("The module \"%s\" does not provide any of the required functions. It will not be loaded."), files[i]));
+            module->Unload();
+         }
+         else
+         {
+            Get().mModules.push_back(std::move(umodule));
+
 #ifdef EXPERIMENTAL_MODULE_PREFS
-         // Loaded successfully, restore the status.
-         ModulePrefs::SetModuleStatus( files[i], iModuleStatus);
+            // Loaded successfully, restore the status.
+            ModulePrefs::SetModuleStatus(files[i], iModuleStatus);
 #endif
+         }
       }
    }
    ::wxSetWorkingDirectory(saveOldCWD);

src/ModuleManager.h

@@ -51,6 +51,7 @@ public:
 
    bool Load();
    void Unload();
+   bool HasDispatch() { return mDispatch != NULL; };
    int Dispatch(ModuleDispatchTypes type);
    void * GetSymbol(const wxString &name);