[Linux] Regression: Crash on REAPER when reopening plugin UI (cairo-xcb assertion)

[yonie]

Description

A regression was introduced in commit 5dc86672 ("cairo_device_finish is automatically called by cairo_device_destroy when the ref count reaches zero") which removed the cairo_device_finish() call from CairoGraphicsDevice::~CairoGraphicsDevice(). This causes a crash on Linux when repeatedly opening/closing a plugin UI in REAPER.

Crash

reaper: ../src/cairo-xcb-screen.c:219: _get_screen_index: Assertion `!"reached"' failed.

Root Cause

The commit 5dc86672 removed this code that was added in PR #337:

// In CairoGraphicsDevice::~CairoGraphicsDevice()
if (impl->device)
{
    cairo_device_finish (impl->device);  // This line was removed
    cairo_device_destroy (impl->device);
}

The commit message claims "cairo_device_finish is automatically called by cairo_device_destroy when the ref count reaches zero", but this doesn't account for the XCB connection lifecycle when the host (REAPER) repeatedly opens/closes the plugin editor.

Workaround

Re-adding cairo_device_finish() before cairo_device_destroy() fixes the crash.

Reproduction

1. Build a VST3 plugin using VSTGUI on Linux
2. Load in REAPER (Linux, Flatpak version tested)
3. Open the plugin UI
4. Close and reopen the plugin UI multiple times
5. Crash occurs on the 2nd-4th reopen

Environment

• REAPER 7.65 (Flatpak)
• Linux (Fedora)
• VSTGUI commit: d473fc80 (origin/develop)

Related

• PR Remove all cairo usage from RunLoop and fix #334 (open/close editor c… #337 - "Remove all cairo usage from RunLoop and fix [Linux] Repeatedly reopening GUI crashes host, asserts in cairo-xcb-screen.c #334 (open/close editor crash)"
• Issue [Linux] Repeatedly reopening GUI crashes host, asserts in cairo-xcb-screen.c #334 - Original crash report

[fadams]

FWIW this is the same regression I raised last week #376

I've put more info in there but the underlying background is the incorrect assumption that ref count reaches zero - which is required for cairo_device_destroy to call cairo_device_finish itself.

As mentioned in my issue report even notwithstanding "internal" cairo references being made (which there are) there are actually attributes in DrawHandler that retain references and those won't go out of scope until after ~CairoGraphicsDevice () is called. So to be "clean"

you'd need (in ~DrawHandler ())

windowSurface.reset();
backBuffer.reset();
drawContext.reset();

before that removeDevice call.

But even that doesn't reduce the refcount enough due to cairo caching and other internal references (IIRC the pango drawing increases refcount too) so that cairo_device_finish really is necessary - along with a comment to prevent it from getting inadvertently removed again :-)