I'm seeing this as well.

I found that the problem was introduced between wine 1.7.6 and 1.7.7.

I'm still trying to find the guilty commit.

Confirming based on comment #1.

The following commit introduced the bug.

from git bisect:
----------------------------------------------------
df6dc091e6524581549d65c04c61f5ccd22dfe48 is the first bad commit
commit df6dc091e6524581549d65c04c61f5ccd22dfe48
Author: Sebastian Lackner <sebastian@fds-team.de>
Date:   Sat Oct 26 18:34:32 2013 +0200

    winex11: Implement additional XEMBED events.

:040000 040000 9e1d4762cc154a715c94518ae0ea5d7b0195e4dd e7de4e4aa37502fd4726b9c4fcb5271834e22e44 M      dlls
----------------------------------------------------

Reverting this commit fixes the bug. I'm now trying to find a suitable patch.

Thanks for adding me to the bug report.

Are you guys using a Windows version of LMMS, or using one of the "wrapper" tools to embed Windows VST plugins directly into the Linux version of LMMS?
This patch basically only adds a few more event handlers which shouldn't be triggered when running Windows-only programs (the rest is just moving around code).

Things you could try out:

* Revert just the code inside handle_xembed_protocol, instead of the whole patch. I can provide a patch later when you're not sure how to do this. If the probl
* Create a log with "WINEDEBUG=+event" - this could probably help to figure out whats going wrong in this case.

Moreover: It would be nice to have the possibility to reproduce this problem - can someone of you guys provide some more details on the exact configuration and which VST plugins you're trying exactly?

This crash is for the linux version embedding Windows VST plugins directly.

Reverting the code handling the XEMBED_EMBEDDED_NOTIFY event within handle_xembed_protocol was enough to aleviate the problem.

The call raising the crash is located in create_foreign_window (A call to XGetWindowAttributes), however, I don't know what went wrong up until that point to cause the crash. Theres bit of code between when the XEMBED_EMBEDDED_NOTIFY event is generated and the offending call is made.

I've attached a log generated using wine 1.7.11 and WINEDEBUG=+event as you suggested.

To test this bug, you can use any VST. They all crash on loading. I've been using this: http://www.yohng.com/files/4fpiano-win.zip because it's free and I like the piano. Open a new project, Drag Vestige under the Instrument plugins tab on the left onto the song editor. Click on the button which says 'Vestige' and click the folder icon to select the VST dll. You can just use the dll produced from unzipping the file above since it doesn't require any installation. The crash should occur when Vestige tries to load the GUI for the plugin.

Created attachment 47283 [details]
Events log from a VST plugin crash

The problem seems to be clear, but I'm not yet sure whats the right way to fix that.

Based on your log the VST wrapper you're using generates an invalid XEMBED event:
trace:event:handle_xembed_protocol win 0x10048/6600003 XEMBED_EMBEDDED_NOTIFY owner 0

Based on the specification the owner should be a valid XWindow handle instead:
http://standards.freedesktop.org/xembed-spec/xembed-spec-latest.html

Is the source of the wrapper somewhere available?

A possible workaround would be to catch such invalid messages, for example by replacing the if clause here:

1664             /* window has been marked as embedded before (e.g. systray) */
1665             if (data->embedded)

with:

if (data->embedded || !data->embedder)

Does this help to workaround the problem?

Hi,

as the original main developer of LMMS and its VST support layer, I'm joining. First of all thanks for your efforts to fix this issue! Regarding your questions:

The source code of the application which is launched externally using WINE can be found here:

https://github.com/LMMS/lmms/blob/stable-0.4/plugins/vst_base/RemoteVstPlugin.cpp

At line 692 you can find the code which retrieves the X11 window ID. This code always has been working. Is there any other recommended method?


Inside LMMS we use QX11EmbedContainer to embed the window inside our MDI sub window:

https://github.com/LMMS/lmms/blob/stable-0.4/plugins/vst_base/VstPlugin.cpp

at line 245 ff. So basically we don't do any magic here.

@ Tobias:

Thanks for providing the links. Concerning the Windows side: this looks fine to me, in fact we're using a similar way for our project Pipelight.

For the Linux side: The QX11EmbedContainer is pretty much broken. We also had several issues when developing Pipelight with this one, and I think their code contains several issues which makes them incompatible with for example the GTK XEmbed containers. While developing Pipelight I took a look at their code (for example: http://qt4-x11.sourcearchive.com/documentation/4.5.3really4.5.2/qx11embed__x11_8cpp_source.html ) and immediately noticed several things which seem to be incorrect, like:

* the QX11EmbedWidget watches for changes in _XEMBED_INFO to hide/show the window. This is incorrect, the specification states that "The embedder must track the flags field [...] on the client and map and unmap the client appropriately" - basically they've implemented it exactly the other way round.

* The code of QX11EmbedContainer::showEvent and QX11EmbedContainer::hideEvent is exactly the same - which should make pretty obvious that one of both is incorrect. In fact both are incorrect, since (as stated above) they've implemented it the wrong way.

I think there were some more problems, but I don't remember them by heart.

Concerning the problem in exactly this case - in their code its:

sendXEmbedMessage(client, q->x11Info().display(), XEMBED_EMBEDDED_NOTIFY, q->internalWinId(), minversion);

where internalWinId() should be the ID of the container, but somehow isn't? I assume that the container is somehow not yet initialized when the window is embedded, but I'll take a closer look at all this later when I'm home.

Nevertheless the workaround above should (most probably) have the effect that it works at least "as good" as in previous versions of Wine. To make the whole XEmbed stuff work better, the easiest solution would probably to switch to GTK ;-)

Thanks for clarifying! Using Gtk is not an option for us (and actually we're quite satisfied with Qt ;-). As they dropped QX11Embed classes in Qt5 (and replaced it by a more generic approach in QWindow) I think it's the best solution to ship LMMS with a forked (and fixed) version of these classes. Would you mind helping us to fix the worst issues (e.g. the XEMBED_EMBEDDED_NOTIFY thing)?

After taking a closer look I have finally figured out whats going wrong:

This is (as I already suspected) a bug in the QT implementation - they have messed up the order of the arguments when they pass them to sendXEmbedMessage() *sic*

I have setup a test environment, and confirmed that the following "workaround" patch solves this problem - feel free to test it yourself, too:
http://source.winehq.org/patches/data/101921

Nevertheless this means that basically the whole XEmbed implementation which recently got upstream into Wine will not be used for VST plugins!

@ Tobias:

If you prefer to have XEmbed working the "right" way I can help you guys to fix the issues in a fork, if you want to. I can provide patches for the most critical issues in there if you give me a few days, but it will most likely not be compatible with "regular" QT applications, that expect the wrong behaviour (except we can get all the bugfixes upstream). You can contact me either via mail or even better just join our channel #pipelight on IRC freenode (nickname: slackner). ;-)

Is it possible to upstream a fix for Qt itself?

@Nikolay:

I have opened a bug report for it:
https://bugreports.qt-project.org/browse/QTBUG-36446

Nevertheless I think my submitted wine patch wouldn't really hurt to stay compatible with systems that are shipped with a broken version. Not sure if/how fast QT pushes out updates. ;-) In never versions (QT >= 5.0) they have removed the whole QX11EmbedWidget completely.

(In reply to comment #13)
> @Nikolay:
> 
> I have opened a bug report for it:
> https://bugreports.qt-project.org/browse/QTBUG-36446

Thanks.

> 
> Nevertheless I think my submitted wine patch wouldn't really hurt to stay
> compatible with systems that are shipped with a broken version. Not sure
> if/how fast QT pushes out updates.

Sure, this change looks minimal.

> ;-) In never versions (QT >= 5.0) they
> have removed the whole QX11EmbedWidget completely.

I have no idea if older versions are still supported, but it's possible they won't accept it for released versions to keep them work as they do now.

This is fixed with http://source.winehq.org/git/wine.git/?a=commit;h=f3568a5e1222ce2a85a05efeb417c141fceb0d78, right?

I can confirm, this patch fixes the issue.

(In reply to comment #16)
> I can confirm, this patch fixes the issue.

Thanks for testing.

My patch fixes the problem by falling back to the previous behaviour in case of broken QX11 implementations.

@Tobias:

If the problems with QT are fixed later you will probably experience an additional problem, which is caused by the fact that you make the window visible before its embedded. This never works that well, and I would recommend to do this after the reparenting step (by using some additional ipc messages).

Closing bugs fixed in 1.7.12.