Fix crash when repeatedly opening/closing GUI

[madah81pnz1]

Cairo keeps a global cache list of connections, but since the reference count of the device never reaches 0, the devices are never removed from this cache list.

The crash happens when a new xcb_connection is made, and the new pointer happens to be the same as an existing pointer in this cache list, then the wrong cairo_xcb_connection is returned. Then no screen can be found, since the root is not the same for that old xcb_connection and the new one. Thus we end up at ASSERT_NOT_REACHED in _get_screen_index(), in cairo-xcb-screen.c

Note that there is some resource leakage somewhere, since the reference count of the cairo devices should reach 0 at some point, but never do.

Fixes #334

[rehans]

Thanks for looking into that @madah81pnz1

I just reproduced the crash. I load a single instance of AGain.vst3 on a track in reaper and rapidly press the 'FX' button to quickly open/close the editor. The stacktrace looks like the one in your issue #334

Unfortunately your fix does not work here as the CairoGraphicsDeviceFactory::Impl destructor is not being hit under the debugger when only opening/closing the editor. CairoGraphicsDeviceFactory::Impl only gets destroyed when you unload the plug-in entirely.

What am I missing here? Are you doing something more than I am?

Note that there is some resource leakage somewhere, since the reference count of the cairo devices should reach 0 at some point, but never do.

Yes, that is correct. The reference count is not null here either.

[madah81pnz1]

Thanks for looking into that @madah81pnz1

I just reproduced the crash. I load a single instance of AGain.vst3 on a track in reaper and rapidly press the 'FX' button to quickly open/close the editor. The stacktrace looks like the one in your issue #334

Unfortunately your fix does not work here as the CairoGraphicsDeviceFactory::Impl destructor is not being hit under the debugger when only opening/closing the editor. CairoGraphicsDeviceFactory::Impl only gets destroyed when you unload the plug-in entirely.

What am I missing here? Are you doing something more than I am?

My plugin is not using vst3editor.cpp though, it's based on https://github.com/surge-synthesizer/clap-saw-demo/blob/main/src/linux-vstgui-adapter.cpp
In my plugin, when the last editor is closed, I call VSTGUI::exit. The next opened editor would then call VSTGUI::init. I guess this is different from how vst3editor.cpp does it, it only calls VSTGUI::exit when the whole plugin is unloaded, not just the last editor or the last plugin instance?

In my case, I could only reproduce the crash when VSTGUI::exit/init was called several times, since that left some dangling pointers in cairo. If I don't call VSTGUI::exit, the crash never happens for me in reaper. For me the crash always happens when the next editor is created.

It is then clear my fix doesn't work. In the linked issue ryukau/VSTPlugins#54 it also crashes sometimes even with my fix.

Note that there is some resource leakage somewhere, since the reference count of the cairo devices should reach 0 at some point, but never do.

Yes, that is correct. The reference count is not null here either.

Do you mean this is by design, or this is one of the bugs?

[rehans]

This commit (66ba211#diff-2867d84eec098746cb067c5c069d48234e851d42c749599f36fbf80bf986c194L175) by @scheffle removed one important line accidentally I assume:

RunLoop::instance ().setDevice (cairo_surface_get_device (s));
...at the end of DrawHandler constructor.

(This is a fix for #249)

@madah81pnz1 Please try to add this line back. This immediately fixes the crash here.

[scheffle]

No, this was not an accident. What's most likely missing now is a cairo_device_finish call in the destructor of CairoGraphicsDevice. This reminds me to clean up the RunLoop instance to not include a cairo_device.
Please test it by modifying the destructor of CairoGraphicsDevice to:

CairoGraphicsDevice::~CairoGraphicsDevice () noexcept
{
	if (impl->device)
	{
		cairo_device_finish (impl->device);
		cairo_device_destroy (impl->device);
	}
}

[ryukau]

Both fixes from @rehans and @scheffle worked on my environment (Reaper on Ubuntu). I haven't tested a case that opening multiple windows from a single plugin instance.