Bug 100947 - The xxxx_set_user_data() API cause client data corrupted in listener callbacks
Summary: The xxxx_set_user_data() API cause client data corrupted in listener callbacks
Status: CLOSED NOTABUG
Alias: None
Product: Wayland
Classification: Unclassified
Component: wayland (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: medium trivial
Assignee: Wayland bug list
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-05 12:52 UTC by Freeman Zhang
Modified: 2017-05-05 23:31 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Freeman Zhang 2017-05-05 12:52:41 UTC
Hello everyone.
I'm developing a simple C++ GUI toolkit based on wayland, and get stuck with a weird problem recently.

I created a simple project with minimal code to reproduce this issue for demonstration: https://github.com/zhanggyb/set-user-data

It's a CMake C++ project and use simple C++ structures to wrap wayland client objects.

My test environment:

    Fedora 25
    Gnome 3.22.2
    wayland-devel-1.12.0-1.fc25.x86_64
    libwayland-client-1.12.0-1.fc25.x86_64

Checkout, build and run:

    $ git clone https://github.com/zhanggyb/set-user-data
    $ mkdir build
    $ cd build/
    $ cmake ..
    $ ./demo

Expected result:

    display a rectangle surface on desktop

Actual result:

    assert error will be raised, and the const member variables around wayland object will be changed in the wl_output or wl_surface listener callbacks, see https://github.com/zhanggyb/set-user-data/blob/master/surface.cpp and https://github.com/zhanggyb/set-user-data/blob/master/output.cpp

    Uncomment these 2 lines can avoid assert error:

    https://github.com/zhanggyb/set-user-data/blob/master/main.cpp:163: output.SetUserData(this);
    https://github.com/zhanggyb/set-user-data/blob/master/main.cpp:164: surface.SetUserData(this);

I don't know why this happens, it seems the compositor changes the data in client, or did I use the xxxx_set_user_data() API in the wrong way?

Thanks in advance.
Comment 1 Daniel Stone 2017-05-05 13:31:07 UTC
In main.cpp, you call 'output.SetUserData(this)' and 'surface.SetUserData(this)', from within an Application class; the Output/Surface listeners then try to access the Application class as an Output or Surface class. Changing it to 'output.SetUserData(&output)' and 'surface.SetUserData(&surface)' instead, fixes the bug.
Comment 2 Pekka Paalanen 2017-05-05 13:32:57 UTC
Hi,

I suppose this is the common mistake: *_add_listener() sets the same user_data member as *_set_user_data(). It's not exactly underlined in the docs, but it is there: https://wayland.freedesktop.org/docs/html/apb.html#Client-classwl__proxy for wl_proxy_add_listener() and wl_proxy_set_user_data().

The compositor has nothing to do with this. If you call *_set_user_data(), you will overwrite the value set by *_add_listener().
Comment 3 Pekka Paalanen 2017-05-05 13:34:55 UTC
Another possible surprise is that that "add_listener" does not actually "add". There is a single slot for a listener and it sets it unless it was already set.
Comment 4 Freeman Zhang 2017-05-05 23:31:44 UTC
Thanks everyone!!


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.