Skip to content

Commit 553c5da

Browse files
committed
[audioplugins] re-validate rediscovered plugins instead of trusting Missing
scanPlugins marked a vanished plugin Missing regardless of its prior state, and a later rediscovery flipped it straight to Validated. A plugin that was Error before going Missing - or one whose binary changed to a broken build while away - would thus be trusted without re-validation. Route a reappeared (formerly Missing) path back through newPluginPaths so the out-of-process validation re-derives Validated/Error, mirroring the Discovered (interrupted-scan) handling. Drops the rediscoveredPluginIds list and the blind setPluginsState(..., Validated) step. Adds scanPlugins classification tests.
1 parent 9121677 commit 553c5da

3 files changed

Lines changed: 34 additions & 50 deletions

File tree

framework/audioplugins/internal/registeraudiopluginsscenario.cpp

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,16 @@ PluginScanResult RegisterAudioPluginsScenario::scanPlugins(Progress* progress) c
8989
continue;
9090
}
9191

92-
// Every formerly Missing ID under this path is rediscovered.
93-
for (const CacheEntry& entry : entries) {
94-
if (entry.state == AudioPluginState::Missing) {
95-
result.rediscoveredPluginIds.push_back(entry.id);
96-
}
92+
// A path we previously marked Missing is back. Re-validate it
93+
// out-of-process rather than trusting the cache — the binary may now
94+
// be a newer build that no longer passes validation. Mirrors the
95+
// Discovered handling above.
96+
const bool hasMissing = std::any_of(entries.cbegin(), entries.cend(),
97+
[](const CacheEntry& e) {
98+
return e.state == AudioPluginState::Missing;
99+
});
100+
if (hasMissing) {
101+
result.newPluginPaths.push_back(path);
97102
}
98103
registered.erase(it);
99104
}
@@ -124,12 +129,6 @@ Ret RegisterAudioPluginsScenario::updatePluginsRegistry()
124129
return ret;
125130
}
126131

127-
ret = knownPluginsRegister()->setPluginsState(result.rediscoveredPluginIds, AudioPluginState::Validated);
128-
if (!ret) {
129-
LOGE() << "Failed to mark rediscovered plugins: " << ret.toString();
130-
return ret;
131-
}
132-
133132
ret = registerNewPlugins(result.newPluginPaths, /*validate*/ true);
134133
if (!ret) {
135134
LOGE() << "Failed to register new plugins: " << ret.toString();

framework/audioplugins/iregisteraudiopluginsscenario.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@
3131

3232
namespace muse::audioplugins {
3333
struct PluginScanResult {
34-
io::paths_t newPluginPaths; // not in cache; will be inserted via subprocess validation
34+
// not in cache, or a previously Missing entry the scanner found again: both
35+
// get (re-)validated via subprocess. A returned plugin is re-validated rather
36+
// than trusted, since the binary on disk may now be a newer, broken build.
37+
io::paths_t newPluginPaths;
3538
PluginResourceIdList missingPluginIds; // in cache but not currently found by any scanner
36-
PluginResourceIdList rediscoveredPluginIds; // previously Missing entries the scanner found again
3739
};
3840

3941
class IRegisterAudioPluginsScenario : MODULE_CONTEXT_INTERFACE

framework/audioplugins/tests/registeraudiopluginsscenariotest.cpp

Lines changed: 20 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,6 @@ TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_Mark
297297
EXPECT_CALL(*m_knownPlugins, setPluginsState(uninstalledPluginIdList, AudioPluginState::Missing))
298298
.WillOnce(Return(make_ok()));
299299

300-
EXPECT_CALL(*m_knownPlugins, setPluginsState(PluginResourceIdList {}, AudioPluginState::Validated))
301-
.WillOnce(Return(make_ok()));
302-
303300
EXPECT_CALL(*m_knownPlugins, unregisterPlugins(_))
304301
.Times(0);
305302

@@ -313,7 +310,7 @@ TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_Mark
313310
EXPECT_TRUE(ret);
314311
}
315312

316-
TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_RediscoverFormerlyMissing)
313+
TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, ScanPlugins_FormerlyMissingFoundAgainIsRevalidated)
317314
{
318315
auto createPluginInfo = [](const io::path_t& path, AudioPluginState state) {
319316
AudioPluginInfo info;
@@ -324,7 +321,7 @@ TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_Redi
324321
return info;
325322
};
326323

327-
// [GIVEN] One Missing entry that gets reinstalled, one untouched Validated entry
324+
// [GIVEN] One Missing entry that reappears, one untouched Validated entry
328325
AudioPluginInfoList knownPlugins;
329326
knownPlugins.push_back(createPluginInfo("/some/path/AAA.vst3", AudioPluginState::Missing));
330327
knownPlugins.push_back(createPluginInfo("/some/path/BBB.vst3", AudioPluginState::Validated));
@@ -346,20 +343,16 @@ TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_Redi
346343
.WillByDefault(Return(foundPluginPaths));
347344
}
348345

349-
// [THEN] Only AAA gets transitioned back to Validated
350-
PluginResourceIdList rediscoveredIds { knownPlugins[0].meta.id };
351-
352-
EXPECT_CALL(*m_knownPlugins, setPluginsState(PluginResourceIdList {}, AudioPluginState::Missing))
353-
.WillOnce(Return(make_ok()));
354-
355-
EXPECT_CALL(*m_knownPlugins, setPluginsState(rediscoveredIds, AudioPluginState::Validated))
356-
.WillOnce(Return(make_ok()));
357-
358-
EXPECT_CALL(*m_knownPlugins, load())
359-
.WillOnce(Return(muse::make_ok()));
346+
// [WHEN] Scanning
347+
const PluginScanResult result = m_scenario->scanPlugins();
360348

361-
Ret ret = m_scenario->updatePluginsRegistry();
362-
EXPECT_TRUE(ret);
349+
// [THEN] The reappeared Missing plugin is queued for out-of-process
350+
// re-validation rather than trusted straight back to Validated — its binary
351+
// may now be a newer build that no longer passes validation. The untouched
352+
// Validated plugin is left alone.
353+
EXPECT_TRUE(muse::contains(result.newPluginPaths, path_t("/some/path/AAA.vst3")));
354+
EXPECT_FALSE(muse::contains(result.newPluginPaths, path_t("/some/path/BBB.vst3")));
355+
EXPECT_TRUE(result.missingPluginIds.empty());
363356
}
364357

365358
TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_MultiPluginBinaryMarksEveryIdMissing)
@@ -403,8 +396,6 @@ TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_Mult
403396

404397
EXPECT_CALL(*m_knownPlugins, setPluginsState(expectedMissing, AudioPluginState::Missing))
405398
.WillOnce(Return(make_ok()));
406-
EXPECT_CALL(*m_knownPlugins, setPluginsState(PluginResourceIdList {}, AudioPluginState::Validated))
407-
.WillOnce(Return(make_ok()));
408399

409400
EXPECT_CALL(*m_knownPlugins, load())
410401
.WillOnce(Return(make_ok()));
@@ -413,7 +404,7 @@ TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_Mult
413404
EXPECT_TRUE(ret);
414405
}
415406

416-
TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_MultiPluginBinaryRediscoversEveryId)
407+
TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, ScanPlugins_FormerlyMissingMultiIdBinaryQueuedOnce)
417408
{
418409
auto createPluginInfo = [](const io::path_t& path, const PluginResourceId& id, AudioPluginState state) {
419410
AudioPluginInfo info;
@@ -445,19 +436,14 @@ TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_Mult
445436
.WillByDefault(Return(foundPluginPaths));
446437
}
447438

448-
// [THEN] BOTH ids transition back to Validated — not just the last one.
449-
PluginResourceIdList expectedRediscovered { "Shell FxA", "Shell FxB" };
450-
451-
EXPECT_CALL(*m_knownPlugins, setPluginsState(PluginResourceIdList {}, AudioPluginState::Missing))
452-
.WillOnce(Return(make_ok()));
453-
EXPECT_CALL(*m_knownPlugins, setPluginsState(expectedRediscovered, AudioPluginState::Validated))
454-
.WillOnce(Return(make_ok()));
439+
// [WHEN] Scanning
440+
const PluginScanResult result = m_scenario->scanPlugins();
455441

456-
EXPECT_CALL(*m_knownPlugins, load())
457-
.WillOnce(Return(make_ok()));
458-
459-
Ret ret = m_scenario->updatePluginsRegistry();
460-
EXPECT_TRUE(ret);
442+
// [THEN] The reappeared binary is queued once for re-validation; the
443+
// subprocess re-derives each id's state. Nothing is trusted back to
444+
// Validated, and nothing is left Missing.
445+
EXPECT_EQ(result.newPluginPaths, paths_t { shellPath });
446+
EXPECT_TRUE(result.missingPluginIds.empty());
461447
}
462448

463449
TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_LeftoverDiscoveredRevalidates)
@@ -491,12 +477,9 @@ TEST_F(AudioPlugins_RegisterAudioPluginsScenarioTest, UpdatePluginsRegistry_Left
491477

492478
// [THEN] The Discovered path is treated as new — registered as a fresh
493479
// placeholder and re-validated via subprocess. It is NOT marked Missing
494-
// (it's still on disk) and it is NOT considered "rediscovered" (that's
495-
// for paths transitioning out of Missing).
480+
// (it's still on disk).
496481
EXPECT_CALL(*m_knownPlugins, setPluginsState(PluginResourceIdList {}, AudioPluginState::Missing))
497482
.WillOnce(Return(make_ok()));
498-
EXPECT_CALL(*m_knownPlugins, setPluginsState(PluginResourceIdList {}, AudioPluginState::Validated))
499-
.WillOnce(Return(make_ok()));
500483

501484
// [THEN] registerNewPlugins writes a Discovered placeholder for the path
502485
// before spawning the subprocess.

0 commit comments

Comments
 (0)