diff --git a/include/element/plugins.hpp b/include/element/plugins.hpp index ba7ed0871..79500610e 100644 --- a/include/element/plugins.hpp +++ b/include/element/plugins.hpp @@ -3,6 +3,10 @@ #pragma once +#include +#include +#include + #include #define EL_PLUGIN_SCANNER_PROCESS_ID "pspelbg" @@ -79,6 +83,12 @@ class PluginManager : public juce::ChangeBroadcaster { /** Returns true if a scan is in progress using the child process */ bool isScanningAudioPlugins(); + /** Cancels a running background scan and waits for it to finish. + + @param timeoutMs maximum time to wait in milliseconds + */ + void stopScanningAudioPlugins (int timeoutMs = 5000); + /** Returns the name of the currently scanned plugin. This value is not suitable for use in loading plugins */ juce::String getCurrentlyScannedPluginName() const; @@ -188,11 +198,13 @@ class PluginManager : public juce::ChangeBroadcaster { JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (PluginManager) }; -class PluginScanner final { +class PluginScanner final : private juce::Thread { public: PluginScanner (PluginManager& manager); ~PluginScanner(); + /** Receives scan events. Callbacks are always delivered on the + message thread. */ class Listener { public: Listener() {} @@ -205,10 +217,14 @@ class PluginScanner final { static const juce::File& getWorkerPluginListFile(); - /** scan for plugins of type */ + /** Starts an asynchronous scan for plugins of type on a background + thread. Returns immediately. Does nothing if a scan is already + in progress. */ void scanForAudioPlugins (const juce::String& formatName); - /** Scan for plugins of multiple types */ + /** Starts an asynchronous scan for plugins of multiple types on a + background thread. Returns immediately. Does nothing if a scan + is already in progress. */ void scanForAudioPlugins (const juce::StringArray& formats); /** Cancels the current scan operation if possible. */ @@ -217,35 +233,83 @@ class PluginScanner final { /** is scanning */ bool isScanning() const; + /** Blocks until the scan completes, pumping the message queue when + called from the message thread so marshalled callbacks are + delivered. + + @param timeoutMs maximum time to wait in milliseconds + @return true if the scan finished, false on timeout + */ + bool waitForScanToFinish (int timeoutMs = 60000); + /** Add a listener */ void addListener (Listener* listener) { listeners.add (listener); } /** Remove a listener */ void removeListener (Listener* listener) { listeners.remove (listener); } - /** Returns a list of plugins that failed to load */ + /** Returns a list of plugins that failed to load. Only valid when + not scanning. */ const juce::StringArray& getFailedFiles() const { return failedIdentifiers; } + /** Returns a message describing why the last scan aborted early, or + an empty string. Set when the scanner process repeatedly could + not be launched or contacted. Cleared when a new scan starts. */ + juce::String getLastScanError() const; + /** Returns the scanner exe to use for out-of-process scanning. */ juce::File scannerExeFile() const noexcept; /** Set a specific scanner exe. */ void setScannerExe (const juce::File& exe) { _scannerExe = exe; } + /** Set the timeout used when launching the scanner process. */ + void setLaunchTimeout (int ms) { launchTimeoutMs = ms; } + + /** Set how long a single plugin may take before the scanner process + is killed and the plugin treated as crashed. */ + void setPerPluginTimeout (int ms) { perPluginTimeoutMs = ms; } + + /** Set how many consecutive scanner-process failures abort the scan. */ + void setMaxConsecutiveFailures (int n) { maxConsecutiveFailures = n; } + private: friend class PluginScannerCoordinator; + + enum class ScanResult { + ok, ///> The worker responded. The result may be empty. + crashed, ///> The worker crashed or hung loading this plugin. + unavailable, ///> The worker could not be launched or contacted. + cancelled ///> The scan was cancelled. + }; + PluginManager& _manager; - std::unique_ptr superprocess; + std::shared_ptr superprocess; juce::ListenerList listeners; - juce::StringArray identifiers, failedIdentifiers; + juce::StringArray failedIdentifiers; juce::KnownPluginList& list; juce::Atomic cancelFlag { 0 }; juce::File _scannerExe; - + juce::StringArray formatsToScan; + std::atomic scanning { false }; + int launchTimeoutMs; + int perPluginTimeoutMs { 90000 }; + int maxConsecutiveFailures { 3 }; + int consecutiveFailures { 0 }; + bool abortedByFailure { false }; + juce::String lastScanError; + juce::CriticalSection stateLock; + + void run() override; + bool shouldAbort() const noexcept; + void resetWorker (bool alsoKill); + void notifyOnMessageThread (std::function fn); void scanAudioFormat (const juce::String& formatName); - bool retrieveDescriptions (const juce::String& formatName, - const juce::String& fileOrIdentifier, - juce::OwnedArray& result); + ScanResult retrieveDescriptions (const juce::String& formatName, + const juce::String& fileOrIdentifier, + juce::OwnedArray& result); + + JUCE_DECLARE_WEAK_REFERENCEABLE (PluginScanner) }; } // namespace element diff --git a/src/application.cpp b/src/application.cpp index 91f2a871d..2c23e63cd 100644 --- a/src/application.cpp +++ b/src/application.cpp @@ -290,6 +290,7 @@ void Application::shutdown() auto& settings (world->settings()); auto& midi (world->midi()); auto* props = settings.getUserSettings(); + plugins.stopScanningAudioPlugins(); // the scan thread reads the properties file plugins.setPropertiesFile (nullptr); // must be done before Settings is deleted srvs.deactivate(); diff --git a/src/pluginmanager.cpp b/src/pluginmanager.cpp index 5b8f0be92..0487df962 100644 --- a/src/pluginmanager.cpp +++ b/src/pluginmanager.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later #include +#include #include #include @@ -22,12 +23,22 @@ #define EL_PLUGIN_SCANNER_READY_ID "ready" #define EL_PLUGIN_SCANNER_START_ID "start" #define EL_PLUGIN_SCANNER_FINISHED_ID "finished" +#define EL_PLUGIN_SCANNER_PROGRESS_ID "progress" #define EL_PLUGIN_SCANNER_DEFAULT_TIMEOUT 24000 // 24 Seconds #include extern char* program_invocation_name; +#if JUCE_WINDOWS +#define WIN32_LEAN_AND_MEAN +#include +#include +#else +#include +#include +#endif + namespace element { using namespace juce; @@ -93,31 +104,124 @@ static void applyBlacklistingsFromDeadMansPedal (KnownPluginList& list) list.addToBlacklist (crashedPlugin); } +static juce::int64 currentProcessId() +{ +#if JUCE_WINDOWS + return static_cast (_getpid()); +#else + return static_cast (getpid()); +#endif +} + +/** Forcibly terminates a process by ID. Used as a last resort on scanner + workers that a misbehaving plugin has left unable to exit on their own + (e.g. a load that never returns while holding the dynamic linker lock). */ +static void terminateProcess (juce::int64 pid) +{ + if (pid <= 0) + return; +#if JUCE_WINDOWS + if (auto handle = OpenProcess (PROCESS_TERMINATE, FALSE, static_cast (pid))) + { + TerminateProcess (handle, 1); + CloseHandle (handle); + } +#else + ::kill (static_cast (pid), SIGKILL); +#endif +} + } // namespace detail //============================================================================== -class PluginScannerCoordinator : public juce::ChildProcessCoordinator +class PluginScannerCoordinator : public juce::ChildProcessCoordinator, + public std::enable_shared_from_this { public: explicit PluginScannerCoordinator (PluginScanner& o) - : owner (o) + : owner (o) {} + + ~PluginScannerCoordinator() {} + + bool isLaunched() const noexcept { return launched.load(); } + + /** Launches the worker process. Must be called on a shared_ptr managed + instance. launchWorkerProcess uses ChildProcessManager on Linux which + is only safe on the message thread, so the launch is marshalled there + when called from the scan thread. + + @param timeoutMs IPC ping timeout handed to the worker connection + @param abortCheck polled while waiting; return true to abandon + @return true if the worker process launched and connected + */ + bool launch (int timeoutMs, std::function abortCheck) { - if (! launchScanner (EL_PLUGIN_SCANNER_DEFAULT_TIMEOUT, 0)) + auto scannerExe = owner.scannerExeFile(); + if (! scannerExe.existsAsFile()) { - o.listeners.call (&PluginScanner::Listener::audioPluginScanFinished); - juce::AlertWindow::showMessageBoxAsync ( - juce::MessageBoxIconType::WarningIcon, - "Plugin Scanner", - "Could not launch plugin scanner."); + Logger::writeToLog ("Failed to launch plugin scanner: exe not found."); + return false; } - } - ~PluginScannerCoordinator() {} + auto doLaunch = [this, scannerExe, timeoutMs]() -> bool { + Logger::writeToLog (String ("launching plugin scanner: ") + scannerExe.getFullPathName()); + return launchWorkerProcess (scannerExe, EL_PLUGIN_SCANNER_PROCESS_ID, timeoutMs, 0); + }; + + if (MessageManager::getInstance()->isThisTheMessageThread()) + { + if (! doLaunch()) + return false; + + if (! waitForWorkerReady (timeoutMs, abortCheck)) + { + killWorkerProcess(); + return false; + } + + return launched = true; + } + + struct LaunchState + { + juce::WaitableEvent done; + std::atomic ok { false }; + }; + + auto state = std::make_shared(); + + MessageManager::callAsync ([state, weak = std::weak_ptr (shared_from_this()), doLaunch]() { + if (auto self = weak.lock()) + state->ok = doLaunch(); + state->done.signal(); + }); + + const auto deadline = Time::getMillisecondCounter() + static_cast (timeoutMs) + 5000; + while (! state->done.wait (50)) + if (abortCheck() || Time::getMillisecondCounter() > deadline) + return false; + + if (! state->ok.load()) + return false; + + // The pipe exists as soon as the process starts, so wait for the + // worker's ready handshake to confirm it actually connected. A lost + // connection after this point means the scanned plugin took the + // worker down; before it, the scanner itself is unavailable. + if (! waitForWorkerReady (timeoutMs, abortCheck)) + { + killWorkerProcess(); // safe: launchWorkerProcess has completed + return false; + } + + return launched = true; + } enum class State { timeout, gotResult, + progress, connectionLost, }; @@ -131,24 +235,64 @@ class PluginScannerCoordinator : public juce::ChildProcessCoordinator { std::unique_lock lock { mutex }; - if (! condvar.wait_for (lock, std::chrono::milliseconds { 50 }, [&] { return gotResult || connectionLost; })) + if (! condvar.wait_for (lock, std::chrono::milliseconds { 50 }, [&] { return gotResult || gotProgress || connectionLost; })) return { State::timeout, nullptr }; - const auto state = connectionLost ? State::connectionLost : State::gotResult; - connectionLost = false; - gotResult = false; + if (connectionLost) + { + connectionLost = false; + gotResult = gotProgress = false; + return { State::connectionLost, nullptr }; + } - return { state, std::move (pluginDescription) }; + if (gotResult) + { + gotResult = gotProgress = false; + return { State::gotResult, std::move (pluginDescription) }; + } + + gotProgress = false; + return { State::progress, nullptr }; } void handleMessageFromWorker (const MemoryBlock& mb) override { const std::lock_guard lock { mutex }; - pluginDescription = juce::parseXML (mb.toString()); + + const auto message = mb.toString(); + if (message.startsWith (EL_PLUGIN_SCANNER_READY_ID)) + { + // "ready:" — the pid enables force-killing a hung worker. + workerPid = message.fromFirstOccurrenceOf (":", false, false).getLargeIntValue(); + workerReady = true; + condvar.notify_one(); + return; + } + + if (message.startsWith (EL_PLUGIN_SCANNER_PROGRESS_ID)) + { + // Heartbeat sent while the worker is still inside a single, + // possibly slow findAllTypesForFile call (e.g. a VST3 shell + // plugin enumerating many housed sub-plugins). + gotProgress = true; + condvar.notify_one(); + return; + } + + pluginDescription = juce::parseXML (message); gotResult = true; condvar.notify_one(); } + /** Kills the worker connection and forcibly terminates the worker + process. A worker hung inside a plugin's load code cannot process the + kill message or exit on its own, so the OS process must be killed. */ + void terminateWorkerProcess() + { + killWorkerProcess(); + detail::terminateProcess (workerPid.exchange (0)); + } + void handleConnectionLost() override { const std::lock_guard lock { mutex }; @@ -165,21 +309,29 @@ class PluginScannerCoordinator : public juce::ChildProcessCoordinator std::unique_ptr pluginDescription; bool connectionLost = false; bool gotResult = false; + bool gotProgress = false; + bool workerReady = false; + std::atomic launched { false }; + std::atomic workerPid { 0 }; - bool launchScanner (const int timeout = EL_PLUGIN_SCANNER_DEFAULT_TIMEOUT, const int flags = 0) + bool waitForWorkerReady (int timeoutMs, const std::function& abortCheck) { - auto scannerExe = owner.scannerExeFile(); - if (! scannerExe.existsAsFile()) + const auto deadline = Time::getMillisecondCounter() + static_cast (timeoutMs); + std::unique_lock lock { mutex }; + + for (;;) { - Logger::writeToLog ("Failed to launch plugin scanner."); - return false; - } + if (condvar.wait_for (lock, std::chrono::milliseconds { 50 }, [&] { return workerReady || connectionLost; })) + { + if (workerReady) + return true; + connectionLost = false; // consumed here: launch failure, not a crash + return false; + } - Logger::writeToLog (String ("launching plugin scanner: ") + scannerExe.getFullPathName()); - return launchWorkerProcess (scannerExe, - EL_PLUGIN_SCANNER_PROCESS_ID, - timeout, - flags); + if (abortCheck() || Time::getMillisecondCounter() >= deadline) + return false; + } } }; @@ -262,7 +414,31 @@ class PluginScannerWorker : public juce::ChildProcessWorker, && (MessageManager::getInstance()->isThisTheMessageThread() || matchingFormat->requiresUnblockedMessageThreadDuringCreation (pd))) { + // findAllTypesForFile can block for a long time with no feedback + // in between (e.g. a VST3 shell plugin such as WaveShell + // enumerating many housed sub-plugins), so send heartbeats to + // let the coordinator tell a slow scan from a wedged one. + std::mutex hbMutex; + std::condition_variable hbCondvar; + bool scanning = true; + + std::thread heartbeat ([this, &hbMutex, &hbCondvar, &scanning] { + std::unique_lock lock { hbMutex }; + while (! hbCondvar.wait_for (lock, std::chrono::seconds (1), [&] { return ! scanning; })) + { + const String msg (EL_PLUGIN_SCANNER_PROGRESS_ID); + sendMessageToCoordinator ({ msg.toRawUTF8(), msg.getNumBytesAsUTF8() }); + } + }); + matchingFormat->findAllTypesForFile (results, identifier); + + { + const std::lock_guard lock { hbMutex }; + scanning = false; + } + hbCondvar.notify_one(); + heartbeat.join(); } return results; @@ -318,6 +494,10 @@ class PluginScannerWorker : public juce::ChildProcessWorker, nf.add (new CLAPProvider()); plugins->addDefaultFormats(); plugins->setPlayConfig (48000.0, 1024); + + logger->logMessage ("[scanner] ready"); + const auto msg = String (EL_PLUGIN_SCANNER_READY_ID) + ":" + String (detail::currentProcessId()); + sendMessageToCoordinator ({ msg.toRawUTF8(), msg.getNumBytesAsUTF8() }); } void handleConnectionLost() override @@ -339,12 +519,22 @@ class PluginScannerWorker : public juce::ChildProcessWorker, //============================================================================== PluginScanner::PluginScanner (PluginManager& manager) - : _manager (manager), + : juce::Thread ("elscan"), + _manager (manager), list (manager.getKnownPlugins()), - _scannerExe (detail::scannerExeFullPath()) {} + _scannerExe (detail::scannerExeFullPath()), + launchTimeoutMs (EL_PLUGIN_SCANNER_DEFAULT_TIMEOUT) +{ + // Force-create the master weak reference on this thread so copies made + // from the scan thread never race its lazy initialization. + juce::WeakReference (this); +} PluginScanner::~PluginScanner() { + cancel(); + stopThread (5000); + masterReference.clear(); listeners.clear(); superprocess.reset(); } @@ -354,14 +544,72 @@ void PluginScanner::cancel() cancelFlag = 1; } -bool PluginScanner::isScanning() const { return superprocess != nullptr; } +bool PluginScanner::isScanning() const { return scanning.load(); } + +bool PluginScanner::shouldAbort() const noexcept +{ + return cancelFlag.get() != 0 || threadShouldExit(); +} + +bool PluginScanner::waitForScanToFinish (int timeoutMs) +{ + const auto deadline = Time::getMillisecondCounter() + static_cast (timeoutMs); + const bool onMessageThread = MessageManager::getInstance()->isThisTheMessageThread(); + + while (isScanning()) + { + if (Time::getMillisecondCounter() >= deadline) + return false; + + if (onMessageThread) + MessageManager::getInstance()->runDispatchLoopUntil (20); + else + Thread::sleep (20); + } + + // Deliver the queued audioPluginScanFinished callback. + if (onMessageThread) + MessageManager::getInstance()->runDispatchLoopUntil (20); + + return true; +} + +juce::String PluginScanner::getLastScanError() const +{ + ScopedLock sl (stateLock); + return lastScanError; +} + +void PluginScanner::resetWorker (bool alsoKill) +{ + if (superprocess != nullptr && alsoKill && superprocess->isLaunched()) + superprocess->terminateWorkerProcess(); + superprocess.reset(); +} + +void PluginScanner::notifyOnMessageThread (std::function fn) +{ + MessageManager::callAsync ([weak = juce::WeakReference (this), fn = std::move (fn)]() { + if (auto* self = weak.get()) + fn (*self); + }); +} -bool PluginScanner::retrieveDescriptions (const String& formatName, - const String& fileOrIdentifier, - OwnedArray& result) +PluginScanner::ScanResult PluginScanner::retrieveDescriptions (const String& formatName, + const String& fileOrIdentifier, + OwnedArray& result) { if (superprocess == nullptr) - superprocess = std::make_unique (*this); + { + superprocess = std::make_shared (*this); + if (! superprocess->launch (launchTimeoutMs, [this]() { return shouldAbort(); })) + { + // Don't kill: an abandoned launch may still be in flight on the + // message thread. Dropping the reference is enough. + resetWorker (false); + return shouldAbort() ? ScanResult::cancelled : ScanResult::unavailable; + } + } MemoryBlock block; MemoryOutputStream stream { block, true }; @@ -369,19 +617,49 @@ bool PluginScanner::retrieveDescriptions (const String& formatName, stream.writeString (fileOrIdentifier); if (! superprocess->sendMessageToWorker (block)) - return false; + { + resetWorker (true); + return ScanResult::unavailable; + } using State = PluginScannerCoordinator::State; + auto deadline = Time::getMillisecondCounter() + static_cast (perPluginTimeoutMs); for (;;) { - if (cancelFlag.get() != 0) - return true; + if (shouldAbort()) + return ScanResult::cancelled; const auto response = superprocess->getResponse(); if (response.state == State::timeout) + { + if (Time::getMillisecondCounter() >= deadline) + { + Logger::writeToLog (String ("plugin scan timed out: ") + fileOrIdentifier); + resetWorker (true); + return ScanResult::crashed; + } + continue; + } + + if (response.state == State::progress) + { + // The worker is still alive and working on this plugin, so + // push the deadline out rather than treating it as wedged. + deadline = Time::getMillisecondCounter() + static_cast (perPluginTimeoutMs); + notifyOnMessageThread ([name = File::createFileWithoutCheckingPath (fileOrIdentifier).getFileName()] (PluginScanner& s) { + s.listeners.call (&Listener::audioPluginScanStarted, name + "…"); + }); continue; + } + + if (response.state == State::connectionLost) + { + Logger::writeToLog (String ("plugin scanner crashed on: ") + fileOrIdentifier); + resetWorker (true); + return ScanResult::crashed; + } if (response.xml != nullptr) { @@ -394,7 +672,7 @@ bool PluginScanner::retrieveDescriptions (const String& formatName, } } - return (response.state == State::gotResult); + return ScanResult::ok; } } @@ -403,7 +681,9 @@ File PluginScanner::scannerExeFile() const noexcept { return _scannerExe; } void PluginScanner::scanAudioFormat (const String& formatName) { detail::applyBlacklistingsFromDeadMansPedal (list); - auto paths (detail::readSearchPath (*_manager.props, formatName)); + auto paths = _manager.props != nullptr + ? detail::readSearchPath (*_manager.props, formatName) + : FileSearchPath(); StringArray identifiers; std::function pluginName = [] (const String& ID) -> juce::String { return ID; }; @@ -426,42 +706,91 @@ void PluginScanner::scanAudioFormat (const String& formatName) identifiers = provider->findTypes (paths, true, false); } - listeners.call (&Listener::audioPluginScanProgress, 0.0f); + notifyOnMessageThread ([] (PluginScanner& s) { + s.listeners.call (&Listener::audioPluginScanProgress, 0.0f); + }); - float step = 1.f; - for (const auto& ID : identifiers) + const auto total = static_cast (identifiers.size()); + for (int i = 0; i < identifiers.size(); ++i) { - if (cancelFlag.get() != 0) + const auto& ID = identifiers.getReference (i); + + const auto reportProgress = [this, i, total]() { + const float progress = static_cast (i + 1) / total; + notifyOnMessageThread ([progress] (PluginScanner& s) { + s.listeners.call (&Listener::audioPluginScanProgress, progress); + }); + }; + + if (shouldAbort()) return; - listeners.call (&Listener::audioPluginScanStarted, pluginName (ID)); + notifyOnMessageThread ([name = pluginName (ID)] (PluginScanner& s) { + s.listeners.call (&Listener::audioPluginScanStarted, name); + }); if (list.getTypeForFile (ID) || list.getBlacklistedFiles().contains (ID)) + { + reportProgress(); continue; + } OwnedArray descriptions; + // Add to the dead-man's-pedal before scanning so the entry survives + // if this plugin takes down the whole application. auto crashed = detail::readDeadMansPedalFile(); crashed.removeString (ID); crashed.add (ID); detail::setDeadMansPedalFile (crashed); - if (retrieveDescriptions (formatName, ID, descriptions)) - { - for (auto* desc : descriptions) - list.addType (*desc); - - // Managed to load without crashing, so remove it from the dead-man's-pedal.. + const auto removeFromPedal = [&crashed, &ID]() { crashed.removeString (ID); detail::setDeadMansPedalFile (crashed); - } + }; - if (descriptions.size() == 0 && ! list.getBlacklistedFiles().contains (ID)) - failedIdentifiers.add (ID); + switch (retrieveDescriptions (formatName, ID, descriptions)) + { + case ScanResult::ok: + consecutiveFailures = 0; + for (auto* desc : descriptions) + list.addType (*desc); + // Managed to load without crashing, so remove it from the dead-man's-pedal.. + removeFromPedal(); + if (descriptions.size() == 0 && ! list.getBlacklistedFiles().contains (ID)) + failedIdentifiers.add (ID); + break; - listeners.call (&Listener::audioPluginScanProgress, - step / static_cast (identifiers.size())); - step += 1.f; + case ScanResult::crashed: + // Leave the ID on the dead-man's-pedal so it gets blacklisted. + consecutiveFailures = 0; + if (! list.getBlacklistedFiles().contains (ID)) + failedIdentifiers.add (ID); + break; + + case ScanResult::unavailable: + // The scanner process itself failed. Not the plugin's fault: + // never blacklist it. + removeFromPedal(); + if (++consecutiveFailures >= maxConsecutiveFailures) + { + abortedByFailure = true; + { + ScopedLock sl (stateLock); + lastScanError = TRANS ("Plugin scanning stopped early because the " + "scanner process could not be started or " + "kept failing."); + } + return; + } + break; + + case ScanResult::cancelled: + removeFromPedal(); + return; + } + + reportProgress(); } } @@ -473,35 +802,59 @@ void PluginScanner::scanForAudioPlugins (const juce::String& formatName) void PluginScanner::scanForAudioPlugins (const StringArray& formats) { - if (! scannerExeFile().existsAsFile()) + if (isThreadRunning() || scanning.load()) return; - detail::setDeadMansPedalFile ({}); + formatsToScan = formats; cancelFlag = 0; + abortedByFailure = false; + consecutiveFailures = 0; + failedIdentifiers.clearQuick(); + { + ScopedLock sl (stateLock); + lastScanError.clear(); + } + + // Set before startThread so isScanning() is true immediately. + scanning = true; + startThread(); +} - for (const auto& format : formats) +void PluginScanner::run() +{ + if (scannerExeFile().existsAsFile()) { - scanAudioFormat (format); - if (cancelFlag.get() != 0) - break; + detail::setDeadMansPedalFile ({}); + + for (const auto& format : formatsToScan) + { + scanAudioFormat (format); + if (shouldAbort() || abortedByFailure) + break; + } + + resetWorker (true); + + auto crashed = detail::readDeadMansPedalFile(); + for (const auto& c : failedIdentifiers) + crashed.add (c); + crashed.removeDuplicates (false); + crashed.removeEmptyStrings(); + detail::setDeadMansPedalFile (crashed); + detail::applyBlacklistingsFromDeadMansPedal (list); + detail::setDeadMansPedalFile ({}); + } + else + { + ScopedLock sl (stateLock); + lastScanError = TRANS ("The plugin scanner executable is missing."); } - superprocess.reset(); cancelFlag = 0; - - auto crashed = detail::readDeadMansPedalFile(); - for (const auto& c : failedIdentifiers) - crashed.add (c); - crashed.removeDuplicates (false); - crashed.removeEmptyStrings(); - detail::setDeadMansPedalFile (crashed); - detail::applyBlacklistingsFromDeadMansPedal (list); - detail::setDeadMansPedalFile ({}); - failedIdentifiers.clearQuick(); // FIXME: this is a workaround that - // prevents the UI from showing to - // many errors about known-crashed - // plugins - listeners.call (&Listener::audioPluginScanFinished); + scanning = false; + notifyOnMessageThread ([] (PluginScanner& s) { + s.listeners.call (&Listener::audioPluginScanFinished); + }); } //============================================================================== @@ -884,6 +1237,15 @@ bool PluginManager::isScanningAudioPlugins() : false; } +void PluginManager::stopScanningAudioPlugins (int timeoutMs) +{ + if (priv != nullptr && priv->scanner != nullptr && priv->scanner->isScanning()) + { + priv->scanner->cancel(); + priv->scanner->waitForScanToFinish (timeoutMs); + } +} + AudioPluginInstance* PluginManager::createAudioPlugin (const PluginDescription& desc, String& errorMsg) { return getAudioPluginFormats().createPluginInstance ( diff --git a/src/ui/pluginmanagercomponent.cpp b/src/ui/pluginmanagercomponent.cpp index cca381821..641ecff37 100644 --- a/src/ui/pluginmanagercomponent.cpp +++ b/src/ui/pluginmanagercomponent.cpp @@ -223,6 +223,11 @@ class PluginListComponent::Scanner : private Timer, startTimer (20); progressWindow.setVisible (true); + // JUCE modality is process-wide: in the plugin version it would also + // block input to other Element instances' editors in the same host. + if (! owner.isPluginVersion()) + progressWindow.enterModalState (true, nullptr, false); + if (! scanner->isScanning()) { if (formatsToScan.size() > 0) @@ -230,10 +235,9 @@ class PluginListComponent::Scanner : private Timer, else scanner->scanForAudioPlugins (formatToScan.getName()); } - else - { - finishedScan(); - } + + // If a scan was already running the listener added above tracks it, + // and audioPluginScanFinished drives finishedScan(). } void finishedScan() @@ -241,8 +245,10 @@ class PluginListComponent::Scanner : private Timer, progressWindow.getButton (TRANS ("Cancel"))->onClick = nullptr; stopTimer(); + String scanError; if (scanner) { + scanError = scanner->getLastScanError(); scanner->removeListener (this); scanner.reset(); } @@ -251,10 +257,14 @@ class PluginListComponent::Scanner : private Timer, progressWindow.setVisible (false); progressWindow.removeFromDesktop(); - MessageManager::getInstance()->runDispatchLoopUntil (14); StringArray failedFiles; // TODO owner.scanFinished (failedFiles); + + if (scanError.isNotEmpty()) + AlertWindow::showMessageBoxAsync (AlertWindow::WarningIcon, + TRANS ("Plugin Scanner"), + scanError); } void timerCallback() override @@ -275,7 +285,6 @@ class PluginListComponent::Scanner : private Timer, { pluginBeingScanned = File::createFileWithoutCheckingPath (pluginName.trim()) .getFileName(); - MessageManager::getInstance()->runDispatchLoopUntil (14); } void audioPluginScanProgress (const float reportedProgress) override @@ -791,6 +800,12 @@ void PluginListComponent::optionsMenuCallback (int result) removeMissingPlugins(); saveSettings (this); break; + case 5: + list.clearBlacklistedFiles(); + plugins.getDeadAudioPluginsFile().deleteFile(); + saveSettings (this); + updateList(); + break; case 99: editPluginPath ("CLAP"); @@ -864,6 +879,7 @@ void PluginListComponent::buttonClicked (Button* button) menu.addItem (2, TRANS ("Remove selected plug-in from list"), table.getNumSelectedRows() > 0); menu.addItem (3, TRANS ("Show folder containing selected plug-in"), canShowSelectedFolder()); menu.addItem (4, TRANS ("Remove any plug-ins whose files no longer exist")); + menu.addItem (5, TRANS ("Clear blacklisted plug-ins"), plugins.getKnownPlugins().getBlacklistedFiles().size() > 0); menu.addSeparator(); menu.addItem (8, "Scan for new or updated CLAP plugins"); for (int i = 0; i < formatManager.getNumFormats(); ++i) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 5e73f1ffe..d883e81ae 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -13,7 +13,48 @@ target_include_directories(test_element ${Boost_INCLUDE_DIRS}) target_compile_definitions(test_element PRIVATE - EL_TEST_SOURCE_ROOT="${CMAKE_SOURCE_DIR}") + EL_TEST_SOURCE_ROOT="${CMAKE_SOURCE_DIR}" + EL_TEST_SCANNER_EXE_PATH="$") + +# Deliberately misbehaving "plugins" for PluginScannerTests: modules that +# crash or hang on load, laid out as VST3s so the scanner worker loads them. +# Opt-in at runtime via EL_TEST_SCANNER_INTEGRATION=1. The $<1:...> generator +# expressions stop multi-config generators appending a per-config directory. +foreach(_kind IN ITEMS crasher hanger) + add_library(el_test_${_kind} MODULE fixture/badplugin.c) + set(_dir "${CMAKE_CURRENT_BINARY_DIR}/badplugins/${_kind}") + if(WIN32) + # Single-file VST3: a renamed DLL. + set_target_properties(el_test_${_kind} PROPERTIES + PREFIX "" OUTPUT_NAME "${_kind}" SUFFIX ".vst3" + LIBRARY_OUTPUT_DIRECTORY "$<1:${_dir}>") + elseif(APPLE) + set(_contents "${_dir}/${_kind}.vst3/Contents") + set_target_properties(el_test_${_kind} PROPERTIES + PREFIX "" OUTPUT_NAME "${_kind}" SUFFIX "" + LIBRARY_OUTPUT_DIRECTORY "$<1:${_contents}/MacOS>") + file(WRITE "${_contents}/Info.plist" +" + + + + CFBundleExecutable${_kind} + CFBundleIdentifiernet.kushview.test.${_kind} + CFBundlePackageTypeBNDL + + +") + else() + set_target_properties(el_test_${_kind} PROPERTIES + PREFIX "" OUTPUT_NAME "${_kind}" SUFFIX ".so" + LIBRARY_OUTPUT_DIRECTORY + "$<1:${_dir}/${_kind}.vst3/Contents/${CMAKE_SYSTEM_PROCESSOR}-linux>") + endif() + add_dependencies(test_element el_test_${_kind}) +endforeach() +target_compile_definitions(el_test_hanger PRIVATE EL_TEST_HANG=1) +target_compile_definitions(test_element PRIVATE + EL_TEST_BADPLUGINS_DIR="${CMAKE_CURRENT_BINARY_DIR}/badplugins") # Register all test suites with CTest add_test(NAME "AtomicValueTests" COMMAND test_element --run_test=AtomicValueTests) @@ -48,6 +89,7 @@ add_test(NAME "NodeTests" COMMAND test_element --run_test=NodeTests) add_test(NAME "OversamplerTests" COMMAND test_element --run_test=OversamplerTests) add_test(NAME "PluginManagerTests" COMMAND test_element --run_test=PluginManagerTests) add_test(NAME "PluginMetadataTests" COMMAND test_element --run_test=PluginMetadataTests) +add_test(NAME "PluginScannerTests" COMMAND test_element --run_test=PluginScannerTests) add_test(NAME "PortListTests" COMMAND test_element --run_test=PortListTests) add_test(NAME "PresetScriptsTest" COMMAND test_element --run_test=PresetScriptsTest) add_test(NAME "PortTypeTests" COMMAND test_element --run_test=PortTypeTests) diff --git a/test/PluginScannerTests.cpp b/test/PluginScannerTests.cpp new file mode 100644 index 000000000..f842b3703 --- /dev/null +++ b/test/PluginScannerTests.cpp @@ -0,0 +1,266 @@ +// SPDX-FileCopyrightText: Copyright (C) Kushview, LLC. +// SPDX-License-Identifier: GPL-3.0-or-later + +#include + +#include +#include +#include + +using namespace element; + +namespace { + +struct RecordingListener : public PluginScanner::Listener +{ + std::atomic finishedCount { 0 }; + void audioPluginScanFinished() override { ++finishedCount; } +}; + +/** Returns the element executable to use for integration tests that spawn a + real scanner worker, or an invalid File when they should be skipped. + + Opt-in: set EL_TEST_SCANNER_INTEGRATION=1 to use the executable from this + build tree, or point EL_TEST_SCANNER_EXE at a specific binary, e.g. + EL_TEST_SCANNER_INTEGRATION=1 ctest --test-dir build -R PluginScannerTests +*/ +static juce::File integrationScannerExe() +{ + const auto exe = juce::SystemStats::getEnvironmentVariable ("EL_TEST_SCANNER_EXE", {}); + if (exe.isNotEmpty()) + return juce::File (exe); + +#ifdef EL_TEST_SCANNER_EXE_PATH + if (juce::SystemStats::getEnvironmentVariable ("EL_TEST_SCANNER_INTEGRATION", "0") == "1") + return juce::File (EL_TEST_SCANNER_EXE_PATH); +#endif + + return {}; +} + +/** Creates a temp directory holding fake (non-loadable) .vst3 files. */ +static juce::File makeGarbageDir (std::initializer_list names) +{ + auto dir = juce::File::createTempFile ("elscan"); + dir.deleteFile(); + BOOST_REQUIRE (dir.createDirectory()); + for (const auto* name : names) + BOOST_REQUIRE (dir.getChildFile (name).replaceWithText ("not a plugin")); + return dir; +} + +/** Builds a PropertiesFile whose VST3 search path is set to searchPath. */ +static std::unique_ptr makeProps (const juce::File& dir, const juce::String& searchPath) +{ + juce::PropertiesFile::Options opts; + opts.storageFormat = juce::PropertiesFile::storeAsXML; + auto props = std::make_unique (dir.getChildFile ("test.settings"), opts); + props->setValue (juce::String (Settings::lastPluginScanPathPrefix) + "VST3", searchPath); + return props; +} + +} // namespace + +BOOST_AUTO_TEST_SUITE (PluginScannerTests) + +/** A missing scanner executable must still finish the scan and must not + blacklist anything. */ +BOOST_AUTO_TEST_CASE (MissingScannerExeFinishesAndDoesNotBlacklist) +{ + PluginManager manager; + manager.addDefaultFormats(); + + std::unique_ptr scanner (manager.createAudioPluginScanner()); + RecordingListener listener; + scanner->addListener (&listener); + scanner->setScannerExe (juce::File ("/path/does/not/exist/element-scanner")); + + scanner->scanForAudioPlugins (juce::StringArray { "VST3" }); + BOOST_REQUIRE (scanner->waitForScanToFinish (10000)); + + BOOST_CHECK_EQUAL (listener.finishedCount.load(), 1); + BOOST_CHECK (! scanner->isScanning()); + BOOST_CHECK (manager.getKnownPlugins().getBlacklistedFiles().isEmpty()); + BOOST_CHECK (scanner->getFailedFiles().isEmpty()); + BOOST_CHECK (scanner->getLastScanError().isNotEmpty()); + + scanner->removeListener (&listener); +} + +/** A scanner executable that launches but never connects is an + infrastructure failure: no plugin may be blacklisted, and the circuit + breaker must abort the scan with an error. This is the regression test + for one failed plugin poisoning the remainder of a scan. */ +BOOST_AUTO_TEST_CASE (BrokenScannerExeNeverBlacklists) +{ + // A real executable that exits immediately without ever connecting. +#if JUCE_WINDOWS + const juce::File brokenExe ("C:\\Windows\\System32\\where.exe"); +#else + const juce::File brokenExe ("/bin/true"); +#endif + if (! brokenExe.existsAsFile()) + return; + + auto tmp = makeGarbageDir ({ "a.vst3", "b.vst3", "c.vst3" }); + auto props = makeProps (tmp, tmp.getFullPathName()); + + PluginManager manager; + manager.addDefaultFormats(); + manager.setPropertiesFile (props.get()); + + if (! manager.isAudioPluginFormatSupported ("VST3")) + return; + + std::unique_ptr scanner (manager.createAudioPluginScanner()); + RecordingListener listener; + scanner->addListener (&listener); + scanner->setScannerExe (brokenExe); + scanner->setLaunchTimeout (1000); + scanner->setPerPluginTimeout (2000); + scanner->setMaxConsecutiveFailures (2); + + scanner->scanForAudioPlugins (juce::StringArray { "VST3" }); + BOOST_REQUIRE (scanner->waitForScanToFinish (30000)); + + BOOST_CHECK_EQUAL (listener.finishedCount.load(), 1); + BOOST_CHECK (manager.getKnownPlugins().getBlacklistedFiles().isEmpty()); + BOOST_CHECK (scanner->getFailedFiles().isEmpty()); + BOOST_CHECK (scanner->getLastScanError().isNotEmpty()); + + scanner->removeListener (&listener); + scanner.reset(); + manager.setPropertiesFile (nullptr); + tmp.deleteRecursively(); +} + +/** Full round trip against the real element executable: worker launches, + completes the ready handshake, scans garbage files without hanging, and + reports them failed without touching innocent state. */ +BOOST_AUTO_TEST_CASE (RealWorkerScansGarbageWithoutHanging) +{ + const auto exe = integrationScannerExe(); + if (! exe.existsAsFile()) + return; + + auto tmp = makeGarbageDir ({ "fake1.vst3", "fake2.vst3" }); + auto props = makeProps (tmp, tmp.getFullPathName()); + + PluginManager manager; + manager.addDefaultFormats(); + manager.setPropertiesFile (props.get()); + + if (! manager.isAudioPluginFormatSupported ("VST3")) + return; + + std::unique_ptr scanner (manager.createAudioPluginScanner()); + RecordingListener listener; + scanner->addListener (&listener); + scanner->setScannerExe (exe); + scanner->setPerPluginTimeout (30000); + + scanner->scanForAudioPlugins (juce::StringArray { "VST3" }); + BOOST_REQUIRE (scanner->waitForScanToFinish (60000)); + + BOOST_CHECK_EQUAL (listener.finishedCount.load(), 1); + BOOST_CHECK (scanner->getLastScanError().isEmpty()); + // Garbage files legitimately fail to load: reported failed and blacklisted. + BOOST_CHECK_EQUAL (scanner->getFailedFiles().size(), 2); + BOOST_CHECK_EQUAL (manager.getKnownPlugins().getBlacklistedFiles().size(), 2); + + scanner->removeListener (&listener); + scanner.reset(); + manager.setPropertiesFile (nullptr); + tmp.deleteRecursively(); +} + +#if defined(EL_TEST_BADPLUGINS_DIR) +/** THE regression test for the reported bug: a plugin that crashes the + worker process must be blacklisted alone, the worker must be relaunched, + and every plugin after it must still be scanned on its own merits. + + The crasher (built by test/CMakeLists.txt from fixture/badplugin.c) is + placed first in the search path via a two-directory FileSearchPath so the + crash happens before the other files are visited. */ +BOOST_AUTO_TEST_CASE (CrashingPluginDoesNotPoisonScan) +{ + const auto exe = integrationScannerExe(); + const juce::File crashDir = juce::File (EL_TEST_BADPLUGINS_DIR).getChildFile ("crasher"); + if (! exe.existsAsFile() || ! crashDir.isDirectory()) + return; + + auto tmp = makeGarbageDir ({ "after1.vst3", "after2.vst3" }); + auto props = makeProps (tmp, crashDir.getFullPathName() + ";" + tmp.getFullPathName()); + + PluginManager manager; + manager.addDefaultFormats(); + manager.setPropertiesFile (props.get()); + + std::unique_ptr scanner (manager.createAudioPluginScanner()); + RecordingListener listener; + scanner->addListener (&listener); + scanner->setScannerExe (exe); + scanner->setPerPluginTimeout (30000); + + scanner->scanForAudioPlugins (juce::StringArray { "VST3" }); + BOOST_REQUIRE (scanner->waitForScanToFinish (120000)); + + BOOST_CHECK_EQUAL (listener.finishedCount.load(), 1); + BOOST_CHECK (scanner->getLastScanError().isEmpty()); + + // The crasher is blacklisted for crashing; the files after it must have + // been scanned by a relaunched worker and judged on their own merits. + // If the crash had poisoned the scan they would be scanner-unavailable + // failures instead, tripping the circuit breaker. + const auto& blacklist = manager.getKnownPlugins().getBlacklistedFiles(); + BOOST_CHECK_EQUAL (scanner->getFailedFiles().size(), 3); + BOOST_CHECK_EQUAL (blacklist.size(), 3); + BOOST_CHECK (blacklist.contains (crashDir.getChildFile ("crasher.vst3").getFullPathName())); + + scanner->removeListener (&listener); + scanner.reset(); + manager.setPropertiesFile (nullptr); + tmp.deleteRecursively(); +} + +/** A plugin that hangs the worker must be killed at the per-plugin timeout, + treated as crashed, and the scan must continue with a fresh worker. */ +BOOST_AUTO_TEST_CASE (HangingPluginTimesOutAndScanContinues) +{ + const auto exe = integrationScannerExe(); + const juce::File hangDir = juce::File (EL_TEST_BADPLUGINS_DIR).getChildFile ("hanger"); + if (! exe.existsAsFile() || ! hangDir.isDirectory()) + return; + + auto tmp = makeGarbageDir ({ "after1.vst3", "after2.vst3" }); + auto props = makeProps (tmp, hangDir.getFullPathName() + ";" + tmp.getFullPathName()); + + PluginManager manager; + manager.addDefaultFormats(); + manager.setPropertiesFile (props.get()); + + std::unique_ptr scanner (manager.createAudioPluginScanner()); + RecordingListener listener; + scanner->addListener (&listener); + scanner->setScannerExe (exe); + scanner->setPerPluginTimeout (3000); + + scanner->scanForAudioPlugins (juce::StringArray { "VST3" }); + BOOST_REQUIRE (scanner->waitForScanToFinish (120000)); + + BOOST_CHECK_EQUAL (listener.finishedCount.load(), 1); + BOOST_CHECK (scanner->getLastScanError().isEmpty()); + + const auto& blacklist = manager.getKnownPlugins().getBlacklistedFiles(); + BOOST_CHECK_EQUAL (scanner->getFailedFiles().size(), 3); + BOOST_CHECK_EQUAL (blacklist.size(), 3); + BOOST_CHECK (blacklist.contains (hangDir.getChildFile ("hanger.vst3").getFullPathName())); + + scanner->removeListener (&listener); + scanner.reset(); + manager.setPropertiesFile (nullptr); + tmp.deleteRecursively(); +} +#endif + +BOOST_AUTO_TEST_SUITE_END() diff --git a/test/fixture/badplugin.c b/test/fixture/badplugin.c new file mode 100644 index 000000000..5c90322f7 --- /dev/null +++ b/test/fixture/badplugin.c @@ -0,0 +1,49 @@ +// SPDX-FileCopyrightText: Copyright (C) Kushview, LLC. +// SPDX-License-Identifier: GPL-3.0-or-later + +/* A deliberately misbehaving "plugin" used by PluginScannerTests to verify + the scanner survives plugins that crash or hang the worker process. + + Built as a VST3-shaped module so the scanner worker's load runs this + code on attach. EL_TEST_HANG selects hanging instead of crashing. */ + +#include + +#if defined(_WIN32) + +#define WIN32_LEAN_AND_MEAN +#include + +BOOL WINAPI DllMain (HINSTANCE instance, DWORD reason, LPVOID reserved) +{ + (void) instance; + (void) reserved; + + if (reason == DLL_PROCESS_ATTACH) + { +#if defined(EL_TEST_HANG) + for (;;) + Sleep (1000); +#else + abort(); +#endif + } + + return TRUE; +} + +#else + +#include + +__attribute__ ((constructor)) static void badplugin_entry (void) +{ +#if defined(EL_TEST_HANG) + for (;;) + sleep (1); +#else + abort(); +#endif +} + +#endif