Skip to content

EffectsBackend: fix crash when AU manifest loading times out #16298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ProfKenUston wants to merge 2 commits into
base: main
Choose a base branch
from
+20 −9
Open

EffectsBackend: fix crash when AU manifest loading times out #16298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2aeb09e
EffectsBackend: fix crash when AU manifest loading times out
ProfKenUston Apr 7, 2026
8e1cf3d
Add VERIFY_OR_DEBUG_ASSERT() to recognize null manifests during debug…
daschuer Aug 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions src/effects/backends/audiounit/audiounitbackend.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,22 @@ EffectBackendType getType() const override {
};

const QList<QString> getEffectIds() const override {
QList<QString> effectIds;

for (NSString* effectId in m_componentsById) {
effectIds.append(QString::fromNSString(effectId));
}

return effectIds;
// Only report effects whose manifest actually finished loading.
// If loadAudioUnitsOfType() timed out, some components in
// m_componentsById may not have a corresponding manifest yet, and
// callers would get a null EffectManifestPointer from getManifest(),
// which crashes downstream (e.g. EffectManifest::sortLexigraphically).
auto locker = lockMutex(&m_mutex);
return m_manifestsById.keys();
}

EffectManifestPointer getManifest(const QString& effectId) const override {
return m_manifestsById[effectId];
auto locker = lockMutex(&m_mutex);
return m_manifestsById.value(effectId);
}

const QList<EffectManifestPointer> getManifests() const override {
auto locker = lockMutex(&m_mutex);
return m_manifestsById.values();
}

Expand All @@ -64,7 +66,7 @@ bool canInstantiateEffect(const QString& effectId) const override {
private:
NSMutableDictionary<NSString*, AVAudioUnitComponent*>* m_componentsById;
QHash<QString, EffectManifestPointer> m_manifestsById;
QMutex m_mutex;
mutable QMutex m_mutex;

void loadAudioUnits() {
qDebug() << "Loading audio units...";
Expand Down
9 changes: 9 additions & 0 deletions src/effects/backends/effectmanifest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ bool EffectManifest::hasMetaKnobLinking() const {

bool EffectManifest::sortLexigraphically(
EffectManifestPointer pManifest1, EffectManifestPointer pManifest2) {
// Defensive: null manifests should no longer occur here, but
// this check is retained to prevent std::sort from crashing if an
// unexpected null pointer ever reaches this comparator. Nulls sort to the end.
VERIFY_OR_DEBUG_ASSERT(pManifest1) {
return false;
}
VERIFY_OR_DEBUG_ASSERT(pManifest2) {
return true;
}

@daschuer daschuer Apr 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code looks a bit scary. Why is a null manifest added in the first place? I think this shall not happen.
The checks here are OK to not crash, but It would be better if hat never happens and we can use VERIFY_OR_DEBUG_ASSERT() here.
Maybe this is now resolved anyway by the mutex fix?

// Sort built-in effects first before external plugins
int backendNameComparision = static_cast<int>(pManifest1->backendType()) -
static_cast<int>(pManifest2->backendType());
Expand Down