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

ZynAddSubFX "Show GUI" broken (Apple) #703

Closed
tresf opened this issue May 10, 2014 · 45 comments
Closed

ZynAddSubFX "Show GUI" broken (Apple) #703

tresf opened this issue May 10, 2014 · 45 comments
Milestone

Comments

@tresf
Copy link
Member

tresf commented May 10, 2014

The ZynAddSubFX instrument plugin plays audio in OSX, however the "Show GUI" button crashes the software.

The crash seems to be caused within the initPlugin() or possibly the reloadPlugin() code although this could be a missing library reference as well. (view initPlugin() code here)

Click here for the list of libraries that load when lmms is executed
Click here for a partial stacktrace of the crash

Some outstanding QSharedMemory bugs:
https://bugreports.qt-project.org/browse/QTBUG-27765
https://bugreports.qt-project.org/browse/QTBUG-5123

I notice fltk isn't in the list when LMMS loads. I believe that is normal, as only the Remote process should load it. If I had a way of launching ZynAddSubFX without LMMS it might shed some more light on this issue as even a cout debug added to the code won't echo to terminal as it's a separate process.

image

@Sti2nd
Copy link
Contributor

Sti2nd commented May 11, 2014

Windows had this problem with Zyn before 1.0.0, think it was in relation with the update of Zyn? Cause you updated Zyn, right? 💭

Perhaps the fix to the problem wasn't a proper fix, but rather a workaround?

@tresf
Copy link
Member Author

tresf commented May 11, 2014

Yeah I researched that thoroughly prior to opening. Toby's comments
suggested it was GCC related, so I don't think that helps this particular
bug. Thanks for keeping it in mind!

@tresf
Copy link
Member Author

tresf commented Jun 4, 2014

FYI, this bug still exists in the stable-1.1 branch. The new version of Zyn does not fix this.

@tresf
Copy link
Member Author

tresf commented Jun 4, 2014

For now, I will recommend disabling the Zyn GUI until this crash can be resolved. This will gray-out the button so that it is no longer clickable on Apple.

Before: (ZynAddSubFx.cpp#L535)

   m_toggleUIButton->setCheckable( true );

After:

   m_toggleUIButton->setCheckable( true );
#ifdef LMMS_BUILD_APPLE
   m_toggleUIButton->setEnabled( false );
#endif

screen shot 2014-06-04 at 1 14 54 pm

@diizy
Copy link
Contributor

diizy commented Jun 5, 2014

On 06/04/2014 08:19 PM, Tres Finocchiaro wrote:

For now, I will recommend disabling the Zyn GUI until this crash can
be resolved. This will gray-out the button so that it is no longer
clickable on Apple.

I'd say it's better to just not include Zyn at all rather than disable
functionality on it. If you disable the GUI, you won't be able to do
anything with it anyway...

Or, you know, fix whatever is causing the crash. Got any backtraces?

@tresf
Copy link
Member Author

tresf commented Jun 5, 2014

It plays well otherwise, so disabling it completely might be overkill as
the presets work just fine.

The back traces are useless to me at this point because I can't figure an
easy way to prevent stripping out debug symbols. I'm sure this is easy I
just don't know how to do it. Any recommendations for doing this? (Is
there an already existing tutorial for this that I missed?)

@diizy
Copy link
Contributor

diizy commented Jun 5, 2014

On 06/05/2014 02:10 PM, Tres Finocchiaro wrote:

It plays well otherwise, so disabling it completely might be overkill as
the presets work just fine.

The back traces are useless to me at this point because I can't figure an
easy way to prevent stripping out debug symbols. I'm sure this is easy I
just don't know how to do it. Any recommendations for doing this? (Is
there an already existing tutorial for this that I missed?)

Try -DCMAKE_BUILD_TYPE=Debug

@tresf
Copy link
Member Author

tresf commented Jun 5, 2014

Ah... much better. Thank you @diizy!

Here is the back-trace:
https://gist.github.com/tresf/1456073bf4227decc7f8

If you know a bit about debugging, can you verify my steps are correct?
https://github.com/tresf/lmms/wiki/Compiling-lmms-OSX#debugging-lmms-osx

On a side note, the macdeployqt tool has a debug library switch as well but expects two versions of the libraries (the stripped version and an non-stripped version with "devel" in the name. As you can expect, this was a no-go since we don't change naming convention when debug is supplied, but the -DCMAKE_BUILD_TYPE=Debug seems to do the trick. 😄

@tresf
Copy link
Member Author

tresf commented Jun 5, 2014

So I can confirm the hang is occurring here:
https://github.com/LMMS/lmms/blob/stable-1.1/include/RemotePlugin.h#L175

I added a fprintf( stderr, "stuck in while loop\n" ); inside the while loop and this is what I get:

image

@diizy
Copy link
Contributor

diizy commented Jun 5, 2014

On 06/05/2014 04:56 PM, Tres Finocchiaro wrote:

Ah... much better. Thank you @diizy https://github.com/diizy!

Here is the back-trace:
https://gist.github.com/tresf/1456073bf4227decc7f8

frame #0: 0x00007fff92deed52 libsystem_kernel.dylibshmget + 10 frame #1: 0x0000000100060e77 lmmsshmFifo(this=0x000000010bff7610) + 215
at RemotePlugin.h:175
frame #2: 0x000000010005fa32 lmmsRemotePlugin [inlined] shmFifo(this=0x000000010bff7610, this=<unavailable>, _in=0x000000010bff7610, _p=<unavailable>, _out=0x00007fff5fbfe738) + 8 at RemotePlugin.h:206 frame #3: 0x000000010005fa2a lmmsRemotePlugin(this=0x000000010bf1fe20)

  • 26 at RemotePlugin.cpp:87

Seems to have something to do with the shmem stuff, I've no idea how
they work...

@diizy
Copy link
Contributor

diizy commented Jun 5, 2014

On 06/05/2014 05:36 PM, Tres Finocchiaro wrote:

So I can confirm the hang is occurring here:
https://github.com/LMMS/lmms/blob/stable-1.1/include/RemotePlugin.h#L175

Just a random idea:

Try adding

#define USE_QT_SHMEM

on line #45, so it's for apple builds too... See if it makes a difference.

@tresf
Copy link
Member Author

tresf commented Jun 5, 2014

Yeah, thanks. I was trying this as you were typing it. 😄

I do remember Toby recommending this at some point, but it throws some new errors that I'm unfamiliar with:

... use of undeclared identifier 'qint8 ...'
... use of undeclared identifier 'getpid ...'
... use of undeclared identifier 'usleep ...'

Googling them recommends this:

#include <unistd.h>

Which seems to compile, but still crashes here:
https://gist.github.com/tresf/9a888c7196c7391ae1a4

@tresf
Copy link
Member Author

tresf commented Jun 5, 2014

So this is strange because that debug suggests it's hanging on wait here:
https://github.com/LMMS/lmms/blob/stable-1.1/include/RemotePlugin.h#L319

But it should never get there because LMMS_BUILD_APPLE sets USE_QT_SEMAPHORES. I'm confused. 😕

@tresf
Copy link
Member Author

tresf commented Jun 5, 2014

Whoops, scratch that, L319 is now L317 in my modified header.

So it's certainly waiting on L317, m_messageSem.acquire();. So the deadlock happens with both scenarios, although I'm not really sure what it is doing or waiting for.

@unfa
Copy link
Contributor

unfa commented Jun 5, 2014

Good to know that, before I started doing workshops...
On 10 May 2014 07:22, "Tres Finocchiaro" notifications@github.com wrote:

The ZynAddSubFX instrument plugin plays audio in OSX, however the "Show
GUI" button crashes the software.

[image: image]
https://cloud.githubusercontent.com/assets/6345473/2935344/0a96be30-d803-11e3-9a38-787c9a0290e9.png

The crash seems to be caused within the initPlugin() or possibly the
reloadPlugin() code.

https://github.com/LMMS/lmms/blob/stable-1.0/plugins/zynaddsubfx/ZynAddSubFx.cpp#L424

Here are the libraries that load when lmms is executed:
https://gist.github.com/tresf/136df4fb632283345976

Here is a partial stacktrace of the crash:
https://gist.github.com/tresf/96dd4d45acbf6917245d


Reply to this email directly or view it on GitHub
#703.

@musikBear
Copy link

and #892

@diizy
Copy link
Contributor

diizy commented Jun 30, 2014

@tresf

Can you check if this fixes the crashing GUI on OSX

#916

@tresf
Copy link
Member Author

tresf commented Jun 30, 2014

Can you check if this fixes the crashing GUI on OSX

@diizy Unfortunately it does not. I tested the change (initializing the exitProgram variable) both with QT_USE_SHMEM and without QT_USE_SHMEM to no avail. It still seems to end up in an endless loop. Thank you for linking your fix and keeping this bug in mind.

I assume your commit fixes the crash on Win32? Great news if it does! 😈

There's always a chance my linked libraries are wrong and causing issues too although I've checked them using Apple's otool -L command and they seem to be pointing to each-other properly.

-Tres

@diizy
Copy link
Contributor

diizy commented Jun 30, 2014

On 06/30/2014 06:09 PM, Tres Finocchiaro wrote:

@diizy https://github.com/diizy I tested the change (initializing
the |exitProgram| variable) both with QT_USE_SHMEM and without
QT_USER_SHMEM to no avail. Thank you for asking.

I assume this fixes the crash on Win32? Great news if it does!
😈

I've no idea if it does. You're the one with access to a windows box...

@tresf
Copy link
Member Author

tresf commented Jun 30, 2014

I've no idea if it does

Sorry, I assumed fixing #892 (Win32 crash) was the original intent of your commit and (perhaps) you were reproducing via Wine. Interesting enough, Wine actually provides excellent errors. I'm posting the results in #892 as I think they will help resolve those issues.

@musikBear
Copy link

does a win 32 newer that 210614 exists?
link and i test
-o-----------

no need i found Tresf'
https://drive.google.com/file/d/0B4PpvIwHd1U0OVZnMVJEeWtUNjQ/edit?usp=sharing

@tresf
Copy link
Member Author

tresf commented Jul 1, 2014

FYI, Zyn GUI is still broken on Win32 so no need to report that again. ;)

Windows installers (32bit/64bit) dated June 30th, 2014:

https://drive.google.com/file/d/0B4PpvIwHd1U0aE1WaklHX0tfMDg/edit?usp=sharing

https://drive.google.com/file/d/0B4PpvIwHd1U0OVZnMVJEeWtUNjQ/edit?usp=sharing

@tresf
Copy link
Member Author

tresf commented Jul 1, 2014

Off topic for this bug report, but Zyn GUI on 1.0.9.1 Win32 has been fixed. Will link the installer in the appropriate thread.

@tresf
Copy link
Member Author

tresf commented Jul 2, 2014

Back to the Apple stuff...

Apple seems to support shmget (POSIX shared memory allocation) but puts some very strict (small size) limitations on how much memory can be allocated which is shedding light on why the QT shared memory libraries support exists (makes me wonder why QT's method isn't default for all systems?)

Since PostgreSQL uses shmget() and hits similar restrictions I wanted to see how far I could get. I was able to change the results but not fix the problem:

(Debugged by adding #include <errorno.h> and placing printf("shmget() - %s\n", strerror(errno));

Before increasing system values:

shmget() - Cannot allocate memory

After increasing system values:

shmget() - Invalid Argument
shmget() - File Exists

So I'm at a crossroads as to the next logical path. Inclined to switch to using QT's shared memory methods and troubleshoot from there although this is progress even if minor...

@tresf
Copy link
Member Author

tresf commented Jul 8, 2014

FYI - Some outstanding QSharedMemory bugs. Not sure if they affect LMMS code:
https://bugreports.qt-project.org/browse/QTBUG-27765
https://bugreports.qt-project.org/browse/QTBUG-5123

@tresf tresf added the bug label Sep 8, 2014
@tresf tresf added this to the 1.2.0 milestone Sep 8, 2014
@tresf tresf modified the milestones: 1.3.0, 1.2.0 Mar 8, 2015
@shaggyfox
Copy link
Contributor

without QT_SHMEM:

shmdt(m_data) is called before the sem_destroy() which will lead to an segmentation fault.

The following patch should fix that issue.

diff -ur lmms-1.1.3.orig/include/RemotePlugin.h lmms-1.1.3/include/RemotePlugin.h
--- include/RemotePlugin.h  2016-02-26 12:49:58.087588000 +0100
+++ include/RemotePlugin.h  2016-02-26 12:50:38.213681000 +0100
@@ -250,9 +250,6 @@

    ~shmFifo()
    {
-#ifndef USE_QT_SHMEM
-       shmdt( m_data );
-#endif
        // master?
        if( m_master )
        {
@@ -264,6 +261,9 @@
            sem_destroy( m_messageSem );
 #endif
        }
+#ifndef USE_QT_SHMEM
+       shmdt( m_data );
+#endif
    }

    inline bool isInvalid() const

tresf added a commit to tresf/lmms that referenced this issue Feb 27, 2016
@tresf
Copy link
Member Author

tresf commented Feb 27, 2016

@bjalfi,

Thanks for the recommendation... I attempted to implement your solution on master branch via tresf@250f91f but was unsuccessful. I still get the hanging pinwheel of death.

image

could not initialize m_dataSem
could not initialize m_messageSem
could not initialize m_dataSem
could not initialize m_messageSem
could not initialize m_dataSem
could not initialize m_messageSem
could not initialize m_dataSem
could not initialize m_messageSem

@shaggyfox
Copy link
Contributor

Hey,

sem_init() seems to be broken on OSX. You might be successful by defining USE_QT_SEMAPHORES on OSX

--- include/RemotePlugin.h  2016-02-26 06:00:20.000000000 +0100
+++ include/RemotePlugin.h  2016-02-29 00:26:54.624782000 +0100
@@ -37,7 +37,7 @@
 #include <cassert>


-#if defined(LMMS_HAVE_SYS_IPC_H) && defined(LMMS_HAVE_SEMAPHORE_H)
+#if defined(LMMS_HAVE_SYS_IPC_H) && defined(LMMS_HAVE_SEMAPHORE_H) && ! defined(__APPLE__)
 #include <sys/ipc.h>
 #include <semaphore.h>
 #else

I don't use OSX so it's not tested ;)

@tresf
Copy link
Member Author

tresf commented Feb 29, 2016

@bjalfi Thanks again for the recommendation. I tried the approach suggested and clicking the Zyn Show GUI button still hangs the software indefinitely.

On a related note, if your patch fixes the problem for another platform, please issue a pull request so that the Mac issues don't hold it up.

On the other hand, if you're still interested in helping this issue, please continue to share your recommendations. 👍 I can also grant access to a the VMs I use for my OS X build environments if interested (this offer stands for anyone interested).

@tresf
Copy link
Member Author

tresf commented Jun 20, 2016

Thanks to some improvements in RemotePlugin, the hard-crash seems to be gone.

@jasp00 perhaps you can help.... I'm successfully starting the new remote process now with the addition of #2861, but now I receive the following error:

RemotePluginClient::shmget: No such file or directory

@jasp00
Copy link
Member

jasp00 commented Jun 21, 2016

The RemotePluginClient::shmget error is related to VST sync. You should have seen
VST sync support disabled in your configuration before.

@tresf
Copy link
Member Author

tresf commented Jun 21, 2016

The RemotePluginClient::shmget error is related to VST sync

I'm confused... I'm referring to ZynAddSubFX... Can you explain?

@jasp00
Copy link
Member

jasp00 commented Jun 22, 2016

The error is only noise. When a remote plug-in client (RemoteVstPlugin or RemoteZynAddSubFx) is created, the initialization with shared memory is tried, which is started in src/core/VstSyncController.cpp, then it falls back on initialization with messages.

@tresf
Copy link
Member Author

tresf commented Jun 22, 2016

@jasp00 ok thanks. So this is what I get with the changes in RemotePluginClient combined with #2861

untitled

QtXmlWrapper::loadXMLfile(): empty data
RemotePluginClient::shmget: No such file or directory
QtXmlWrapper::loadXMLfile(): empty data
Starting Audio: NULL
Audio Started
Starting MIDI: NULL
MIDI Started
RemotePlugin::DebugMessage: failed getting shared memory
QtXmlWrapper::loadXMLfile(): empty data

<starts here>
WARNING - too late
WARNING - too late
WARNING - too late
WARNING - too late
WARNING - too late
WARNING - too late
WARNING - too late
WARNING - too late
<repeats>

@jasp00
Copy link
Member

jasp00 commented Jun 22, 2016

The output looks normal. The WARNING - too late messages come from ZynAddSubFX null back end. ZynAddSubFX is probably busy doing something else. If the GUI does not show up, you should debug initPlugin().

@expenses
Copy link

As of LMMS 1.2.0-rc3, the 'Show GUI' button doesn't seem to do anything on OSX. What needs to be done to get it to work?

@tresf
Copy link
Member Author

tresf commented Jun 15, 2017

What needs to be done to get it to work?

@jasp00's comments explain what needs to be done.

As of LMMS 1.2.0-rc3, the 'Show GUI' button doesn't seem to do anything on OSX.

It never did anything, we just finally re-enabled it because the crash is gone. The underlying issue still remains. We don't have many people on Mac or those with the experience to troubleshoot remote plugins, so this issue will remain open until someone can step through the code.

If you or someone you know is interested in the code, we can point you in that direction. Specifically, the LMMS executable tries to open Zyn as a remote process and that is something that may depend on linking and/or a proper path to the RemoteZyn process. It's all spelled out in the code if you're interested in helping, we can link the relevant parts.

@lukas-w lukas-w added the macos label Aug 29, 2017
@tresf tresf changed the title ZynAddSubFX "Show GUI" crashes LMMS (Apple) ZynAddSubFX "Show GUI" broken (Apple) Dec 10, 2017
@tresf
Copy link
Member Author

tresf commented Dec 21, 2017

From Discord, @PhysSong and I took a look at the backtrace from RC5, and this is our findings...

tresf - Today at 12:56 AM
@PhysSong how are you with threading?
I spent some more time looking into the Zyn GUI / Apple bug (#703) and it seems to be triggered due to fltk trying to open a Window that doesn't live on the main thread.
[backtrace]

[...]

PhysSong - Today at 1:53 AM
Fl_Window::show() calls Fl_X::make(), and it calls fl_open_display() again.
And It's called here:

Then how about running GUI loop in main thread and message dispatcher in separate thread? Current implementation runs message dispatcher in main thread and use a separate thread for GUI event handling.

@PhysSong
Copy link
Member

@tresf
Copy link
Member Author

tresf commented Dec 21, 2017

@PhysSong thanks! Yes, that fixes it on 10.8 at least. I'll try 10.13 later and report back.

image

@musikBear
Copy link

@tresf -you ned to go deeper. changing the waveform has been a crash issue in ealier releases. I have not yet installed rc5, but will do tomorrow at home

@tresf
Copy link
Member Author

tresf commented Dec 21, 2017

@musikBear I assume you're talking about #1001 (comment), #1991 (comment).

Those are separate, unrelated issues. From memory they only crashed Windows XP. The upside is if this is caused by an obscure threading issue, this patch may help, but speculating won't help. The next release will have this fix applied and you can test it then.

On a side note, please improve the quality of your posts. Although it was done with good intent, it lacks any useful quality that we'd expect from a tester.

  1. Reference the issue you describe by link
  2. Explain that the reference isn't related but offer to test
  3. Don't tell a developer what to do, such as "go deeper". Loading the UI on MacOS is a milestone we've never reached. Diluting it with your undocumented personal agenda can come off as rude.
  4. Try to post details that are relevant. You installing RC5 tomorrow is not remotely relevant to Zyn UI crashing on Apple. Instead, it's just more to read for Apple users following this thread. It doesn't help a sufferer of this bug; It's not in any relevant context of the crash. <3

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

No branches or pull requests