<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, May 9, 2014 at 1:37 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br>
> Looking at it in general, there is one more fun complication.<br>><br>> If the inert object has requests in its interface, that create new<br>
> objects, the server cannot just ignore those requests. I think the<br>
> server will need to actually create new objects if requested, but<br>> make those inert too. Otherwise there would again be a mismatch<br>
> between the server and the client on which objects exist.<br><br></div><div class="gmail_quote">I was hoping that this wasn't actually going to be a problem, but on wl_seat it is.  The server can destroy a seat while a client is calling wl_seat.get_pointer for instance.  Then, yes, we do have to create an intert object.<br>
</div><div class="gmail_quote"><br>On Fri, May 9, 2014 at 9:02 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="">On Fri, 09 May 2014 14:33:58 +0100<br>
Neil Roberts <<a href="mailto:neil@linux.intel.com">neil@linux.intel.com</a>> wrote:<br>
<br>
> Perhaps we should consider applying the patch anyway even though it's<br>
> not ideal. Currently if a client uses a dead output in a request such as<br>
> xdg_surface.set_output Weston will end up with a weston_output pointer<br>
> that points to freed memory. This could cause the compositor to crash.<br>
<br>
</div>True, it's very bad now.<br>
<div class=""><br>
> That is worse than the alternative provided by this patch which is to<br>
> make the client abort. The clients know about the output being destroyed<br>
> via the wl_registry.global_remove event so in practice they would only<br>
> hit the problem in the unlikely event that they used the output in a<br>
> request in the short time between the output being unplugged and<br>
> noticing the removal event.<br>
<br>
</div>That is also true, but if it is fixed improperly now, there is a very<br>
good chance we just forget about the problem, and never fix it for<br>
real. Especially when it becomes very hard to trigger.<br>
<br>
At least make sure we have an open bug report about it, please.<br></blockquote><div><br></div><div>I agree.  This is bad and we need to make sure it gets fixed properly.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

<div class=""><br>
> In the longer term I was thinking maybe it would be good to handle the<br>
> inert resource idea within libwayland-server. We could add a function<br>
> like wl_resource_zombify() which would mark the resource as a zombie and<br>
> call the destroy handlers. From the compositor's perspective it can then<br>
> act as if the resource has been destroyed. We could detect zombie<br>
> resources being used within the request marshalling code and ignore the<br>
> request. If the request creates new resource we can internally create<br>
> new zombie resources too and Weston would never need to know about it. I<br>
> am planning to experiment with this approach now.<br>
<br>
</div>Hmm... will be interesting to see, if that works out. It does feel like<br>
quite a lot of magic in libwayland-server, while also making life a lot<br>
easier.<br></blockquote><div><br></div><div>Most of the magic there is in allowing resources with no handler in libwayland-server.  The patch would be about 4 lines.  Right now, client-side wl_proxy objects are allowed to have a NULL implementation and there's no problem;  server-side, this is not currently allowed.  If we allowed this ten Neil's wl_resource_zombify would only have to set the destructor, and implementation to NULL.  For that matter, we could just do that explicitly in weston and not add API for it.<br>
<br></div><div>The big chunk of magic is in handling new_id requests.  We would need to create zombie wl_resource's for each new_id argument on a call to a zombified wl_resource.  Now that Pekka mentioned it, I'm not sure that we're handling those correctly client-side if there's no implementation set.<br>
</div><div><br></div><div>--Jason Ekstrand<br></div></div></div></div>