On Tue, Nov 19, 2013 at 07:38:52AM +0200, Kalle Vahlman wrote: > 2013/11/19 Bryce W. Harrington <[email protected]>: > > On Fri, Nov 15, 2013 at 08:51:50PM -0800, Kristian Høgsberg wrote: > >> The server requires clients to only allocate one ID ahead of the previously > >> highest ID in order to keep the ID range tight. Failure to do so will > >> make the server close the client connection. However, the way we allocate > >> new IDs is racy. The generated code looks like: > >> > >> new_proxy = wl_proxy_create(...); > >> wl_proxy_marshal(proxy, ... new_proxy, ...); > >> > >> If two threads do this at the same time, there's a chance that thread A > >> will allocate a proxy, then get pre-empted by thread B which then allocates > >> a proxy and then passes it to wl_proxy_marshal(). The ID for thread As > >> proxy will be one higher that the currently highest ID, but the ID for > >> thread Bs proxy will be two higher. But since thread B prempted thread A > >> before it could send its new ID, B will send its new ID first, the server > >> will see the ID from thread Bs proxy first, and will reject it. > >> > >> We fix this by introducing wl_proxy_marshal_constructor(). This > >> function is identical to wl_proxy_marshal(), except that it will > >> allocate a wl_proxy for NEW_ID arguments and send it, all under the > >> display mutex. By introducing a new function, we maintain backwards > >> compatibility with older code from the generator, and make sure that > >> the new generated code has an explicit dependency on a new enough > >> libwayland-client.so. > >> > >> A virtual Wayland merit badge goes to Kalle Vahlman, who tracked this > >> down and analyzed the issue. > >> > >> Reported-by: Kalle Vahlman <[email protected]> > > > > Is there a test case available for checking this problem? > > > > Also, is it a theoretical issue or does it crop up in actual use? If > > the latter, then might be worth including a test case in the test > > suite. > > This has been spotted in a real system with threaded rendering. > Reproducing the issue there was a bit of a challenge, as anything from > the simplest printf to a full tracing would increase the amount of > iterations required for this to trigger. Each iteration took 10-15s > and the iteration count with debug prints soared to hundreds or > thousands... You get the idea. > > However, reproducing the issue with an artificial example is very easy: > > a = wl_proxy_create(...); > b = wl_proxy_create(...); > wl_proxy_marshal(..., b); > wl_proxy_marshal(..., a); > wl_display_flush(...); > > I'm not sure if creating a test case for the artificial case is worth > it though, this patch fixes things so that we can not end up with that > sequence.
Were you able to verify that the patch fixes the problem for you? Kristian _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
