diff --git a/CHANGELOG.md b/CHANGELOG.md index 5975e71c..62a4c07b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,14 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed +- In certain rare circumstances, closing a plugin editor would trigger an X11 + error and crash the Wine plugin host, and with that likely the entire DAW. + This happened because Wine would try to destroy a window that had already been + destroyed. I've seen this happen in Renoise and to a lesser degree in REAPER + with plugins that take a while to close their editors, such as the iZotope Rx + plugins. We now explicitly reparent the window to back the root window first + before deferring the window closing. This should work around the issue, while + still keeping editor closing nice and snappy. - _PSPaudioware InifniStrip_ would fail to initialize because the plugin expects the host to always be using Microsoft COM and it won't initialize it by itself. InfiniStrip loads as expected now. diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 558255cc..4b97c31a 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -114,10 +114,25 @@ xcb_window_t get_x11_handle(HWND win32_handle); */ ATOM get_window_class(); -DeferredWindow::DeferredWindow(MainContext& main_context, HWND window) - : handle(window), main_context(main_context) {} +DeferredWindow::DeferredWindow(MainContext& main_context, + std::shared_ptr x11_connection, + HWND window) + : handle(window), + main_context(main_context), + x11_connection(x11_connection) {} DeferredWindow::~DeferredWindow() { + // NOTE: For some rason, Wine will sometimes try to delete a window twice if + // the parent window no longer exists. I've only seen this cause + // issues with plugins that hang when their window is hidden, like the + // iZotope Rx plugins. In Renoise this would otherwise trigger an X11 + // error every time you close such a plugin's editor, and in other + // DAWs I've also seen it happen from time to time. + const xcb_window_t wine_window = get_x11_handle(handle); + const xcb_window_t root_window = + get_root_window(*x11_connection, wine_window); + xcb_reparent_window(x11_connection.get(), wine_window, root_window, 0, 0); + // XXX: We are already deferring this closing by posting `WM_CLOSE` to the // message loop instead of calling `DestroyWindow()` ourselves, but we // can take it one step further. If we post this message directly then @@ -156,6 +171,7 @@ Editor::Editor(MainContext& main_context, // expect) and also causes mouse coordinates to be relative to the window // itself. win32_window(main_context, + x11_connection, CreateWindowEx(WS_EX_TOOLWINDOW, reinterpret_cast(get_window_class()), "yabridge plugin", @@ -288,7 +304,7 @@ Editor::Editor(MainContext& main_context, // As explained above, we can't do this directly in the initializer // list win32_child_window.emplace( - main_context, + main_context, x11_connection, CreateWindowEx(WS_EX_TOOLWINDOW, reinterpret_cast(get_window_class()), "yabridge plugin child", WS_CHILD, CW_USEDEFAULT, diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 38c57fac..6b9015bd 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -78,9 +78,13 @@ class DeferredWindow { * * @param main_context This application's main IO context running on the GUI * thread. + * @param x11_connection The X11 connection handle we're using for this + * editor. * @param window A `HWND` obtained through a call to `CreateWindowEx` */ - DeferredWindow(MainContext& main_context, HWND window); + DeferredWindow(MainContext& main_context, + std::shared_ptr x11_connection, + HWND window); /** * Post a `WM_CLOSE` message to the `handle`'s message queue as described @@ -92,6 +96,7 @@ class DeferredWindow { private: MainContext& main_context; + std::shared_ptr x11_connection; }; /** @@ -222,7 +227,7 @@ class Editor { */ void do_xembed() const; - std::unique_ptr x11_connection; + std::shared_ptr x11_connection; /** * The Wine window's client area, or the maximum size of that window. This