Remove image on buffer destroy callback only

[psaavedra]

view-backend-exportable-fdo-egl.cpp relies on the bufferDestroyListenerCallback for destroying the image, This is called during the wl_resource_destroy() to release the image.

Related-to: #175 #176

[psaavedra]

Valgrind's traces on #175

==17==  Address 0x1fac1fd0 is 0 bytes inside a block of size 56 free'd
==17==    at 0x48438AF: operator delete(void*) (vg_replace_malloc.c:923)
==17==    by 0x1A8674F5: wl_priv_signal_final_emit (wayland-server.c:2221)
==17==    by 0x1A8674F5: destroy_resource (wayland-server.c:724)
==17==    by 0x1A867C24: wl_resource_destroy (wayland-server.c:744)
==17==    by 0x1A099FE4: ffi_call_unix64 (unix64.S:101)
==17==    by 0x1A0993F5: ffi_call_int (ffi64.c:669)
==17==    by 0x1A86D2A1: wl_closure_invoke (connection.c:1025)
==17==    by 0x1A868216: wl_client_connection_data (wayland-server.c:437)
==17==    by 0x1A86B019: wl_event_loop_dispatch (event-loop.c:1027)

shows that the first deletion happens during the destruction of the wl_client because a problem in the wl_client_connection_data().

On

static int
wl_client_connection_data(int fd, uint32_t mask, void *data)
{
...
        if ((resource_flags & WL_MAP_ENTRY_LEGACY) ||
            resource->dispatcher == NULL) {
            wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER,
                      object, opcode, client);

this part of the code is reached if resource->dispatcher == NULL) so probably this is
because the peer on the channel is already gone at than point and wayland decides to call wl_closure_invoke().

[aperezdc]

I can take a look at this later today, but @carlosgcampos and/or @zdobersek may want to comment here as well.

[alexgcastro]

We are already reviewing the patch, hopefully we will have a resolution this week. Without the option to reproduce the original problems it took some time to check the lifecycle of the object considering that image structure position between wayland and the backend.

[alexgcastro]

The code apparently missed the situation where wayland destruction and the releaseImage happened at the same time. The places in the code that are checking if bufferResource is NULL are doing the work but was not enough, that explains previous patches. Still the code needs a better name and check for that situation, but this patch is in the right direction. We have to handle the image in the desctruction and avoid any operation when the bufferResource is gone. The option implemeted was keep the image and hope for the best.

It is a pity we can not reproduce the original valgrind stacks and crashes to verify the whole situation though.

Thanks for the patch, hopefully we can refactor this code at some point.

[aperezdc]

Thanks a lot @alexgcastro! 🤗

[aperezdc]

Cherry-picked into the wpebackend-fdo-1.14 branch as commit 0d6a75a.