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.

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

crash while storing preset #1199

Closed
spurkopf opened this issue Sep 21, 2019 · 13 comments
Closed

crash while storing preset #1199

spurkopf opened this issue Sep 21, 2019 · 13 comments
Milestone

Comments

@spurkopf
Copy link

When storing an existing user-preset host crashes. Storing an new preset works.

  • Surge Version: Surge-linux-x64-NIGHTLY-2019-09-21
  • Plugin type (VST2, 3 or AU) vst2 + lv2
  • Bits (32/64) 64

Desktop (please complete the following information):

  • OS: [e.g. windows 10] Opensuse Tumbleweed
  • Host [e.g. bitwig, logic] Traction Waveform (freezes) Qtractor (crashes)
@baconpaul
Copy link
Collaborator

By “existing user preset” you mean you save a preset, chose store, edit it, and chose store again with the same name and category? Or you mean you edit a factory preset and save it as a new preset?

Also did you happen to get a stack trace out of either of your hosts?

Thank you

@spurkopf
Copy link
Author

spurkopf commented Sep 21, 2019

Yes, the first one. No need to edit it, just save it again and it happens.
Sorry I couldn´t produce now useful stack-trace till now.

Thanks for investigation.

@baconpaul
Copy link
Collaborator

Ok I’ll take a peek when I’m next at my laptop. Thanks!

@tank-trax do you see this behavior in Linux?

@spurkopf
Copy link
Author

Tested with Surge-linux-x64-1.6.1.1 - there it works.

@baconpaul
Copy link
Collaborator

Ahh I have a theory. Hold on. Booting up my linux VM.

@baconpaul baconpaul added this to the 1.6.2.1 milestone Sep 21, 2019
@baconpaul
Copy link
Collaborator

OK so between 1.6.1.1 and 1.6.2 we added a code path that prompts you before you overwrite on linux and that uses zenity to prompt for ok/cancel with zenity --question

Can you confirm for me that you do or don't have zenity installed?

Let me tag @jpcima also who wrote the code but I bet I can fix it too just by simulating a missing zenity if JP's not online. But I can super easily reproduce your crash by renaming zenity to qenity in the C++. Then we get a core dump right away.

Let me tag this one for 'immediate fix' in our 1.6.2.1 release. Thank you for the report.

@baconpaul
Copy link
Collaborator

OK I've confirmed that if you don't have zenity the execlp falls through and then you get an immediate crash it seems in the child process trying to do some UI thing (my crash backtrace is that the platform xcb handle is bad; but parent has not it seems returned from waitpid so I think it is things going very wrong when excclp can't replace the child process)

Here's a couple of solutions

  1. @jpcima or @falkTX can we do anything more defensive in the vfork/exec pattern
  2. At startup on linux I could check if zenity is in your path with a system call and if it isn't globaly disable it in user interactions
  3. @spurkopf immediately, you could install zenity :) There are functions in it we do require. But we shouldn't core out if it isn't there.

Thoughts folks?

@falkTX
Copy link
Contributor

falkTX commented Sep 21, 2019

Replace the exit call with _exit and the crash will go away.
exit will cleanup resources that are also present in the parent process, and things are undefined after this. we need to use _exit instead.

baconpaul added a commit to baconpaul/surge that referenced this issue Sep 21, 2019
If Zenity was missing, our vfork pattern led to a core.
Really we want zenity installed, but really really we don't want
a core. So scan the path once to see if we have it when we
first bootstrap user interactions

Addresses surge-synthesizer#1199
@baconpaul
Copy link
Collaborator

Oh super useful. let me check that!

baconpaul added a commit to baconpaul/surge that referenced this issue Sep 21, 2019
Per comment from @falkTX on surge-synthesizer#1199 this will stop our missing-zenity-core
baconpaul added a commit to baconpaul/surge that referenced this issue Sep 21, 2019
Per comment from @falkTX on surge-synthesizer#1199 this will stop our missing-zenity-core
baconpaul added a commit that referenced this issue Sep 21, 2019
Per comment from @falkTX on #1199 this will stop our missing-zenity-core
@baconpaul
Copy link
Collaborator

OK @spurkopf I just merged the fix @falkTX suggested to master, along with one other tweak to "fail open". If you build from source you can just pull master and recompile. If you use the built .deb assets it will take about 30 minutes or so for a new nightly to ship. This fix is the first commit today, so any nightly dated Sep-21 or later should be good to go.

Please do let us know if this stops your crash.

But also: some features do require zenity currently. So once you confirm the crash fix, if it is possible on your system, installing zenity in your path will improve your surge user experience.

Best,

@spurkopf
Copy link
Author

That was fast! Yes, I can confirm that this is fixed. Thank you very much baconpaul and falktx!

Unfortunately installing zenity didn´t improve my surge user experience ... it made it crash again. I write a report when I am through it.

@baconpaul
Copy link
Collaborator

Ok please let us know when you have more visibility in your zenith issue

Does “zenity —question” work in a shell for you? And does your daw have a peculiar path? Those are two things which come to mind.

@baconpaul
Copy link
Collaborator

This fix shipped to the release version with todays 1.6.2.1 release. Closing issue. If you find the other crash please do open a new issue or re-open this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants