Skip to content
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

Jump to bottom

Crash: (UI Menu) File -> Load Native Plugin Preset (with no plugin loaded) #40

Closed
NatureIsFrequency opened this issue Feb 6, 2024 · 0 comments
Closed

Crash: (UI Menu) File -> Load Native Plugin Preset (with no plugin loaded) #40

NatureIsFrequency opened this issue Feb 6, 2024 · 0 comments

Comments

@NatureIsFrequency
Copy link
Contributor

Repro steps:

  1. Launch clap-host with no plugin
  2. File -> Load Native Plugin Preset
  3. Crash

Reason for the crash:

PluginHost::loadNativePluginPreset doesn't guard against _plugin being nullptr

Proposed fix:

Have a local patch for this which I'll pull request and reference this issue

Further details:

Image for how to reproduce:
LoadNativePluginPresetCrash

Crash report:
clap-host-2024-02-06-221408.txt
@NatureIsFrequency NatureIsFrequency mentioned this issue Feb 6, 2024
Closed
@NatureIsFrequency NatureIsFrequency changed the title Crash: File -> Load Native Plugin Preset (with no plugin loaded) Crash: (UI Menu) File -> Load Native Plugin Preset (with no plugin loaded) Feb 6, 2024
NatureIsFrequency added a commit to NatureIsFrequency/clap-host that referenced this issue Feb 7, 2024
@NatureIsFrequency
Fix for free-audio#40
5f86d60 
Fix for free-audio#40

Guard against null _plugin within PluginHost::loadNativePluginPreset
Follow error convention in source code

Tested arch: arm64
Confirm no longer crash
@NatureIsFrequency NatureIsFrequency mentioned this issue Feb 7, 2024
Merged
abique added a commit that referenced this issue Feb 7, 2024
@abique
Merge pull request #43 from NatureIsFrequency/Fix_Issue_#40
4c4c7f1 
Fix for #40
@Trinitou Trinitou mentioned this issue Feb 9, 2024
Closed
NatureIsFrequency added a commit to NatureIsFrequency/clap-host that referenced this issue Feb 9, 2024
@NatureIsFrequency
Enable/Disable Menu items based on plugin loaded
1410bb3 
Per @Trinitou suggestions, a plugin specific menu item should only be enabled (available to execute by user)

This fixes the underlying cause of the crashes reported in free-audio#40 and free-audio#42

Therefore this also reverts the now obsolete change in previous merged pull request free-audio#43

Upon load change we update the menu items

Tab indentation set to 3 to be consistent with existing code

Note: Follows the QT memory ownership model, and the QAction pointers lifetime is handled by their parents

Testing:
Launch with no plugin loaded, see menu items are greyed out (disabled), user cannot select these items (as intended) therefore code is not executed and app doesn't crash

Launch with plugin loaded, see menu items are available and executed when user clicks, and confirm still no crashes

Images taken for testing
@NatureIsFrequency NatureIsFrequency mentioned this issue Feb 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@NatureIsFrequency