<div dir="ltr">Hi Hyun Kook,<div class="gmail_extra"><br><div class="gmail_quote">On 23 September 2016 at 00:40, Hyun Kook Khang <span dir="ltr"><<a href="mailto:hyunkook.khang@lge.com" target="_blank">hyunkook.khang@lge.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>in wl_client_connection_data(),</div><div>there is no guarantee that client would never be destroyed in the process of handling the pending input.</div><div><br></div><div>For example,</div><div>1. It comes to wl_client_connection_data()</div><div>2. wl_closure_invoke() would be invoked, and then wl_display_flush_clients would be invoked anyhow later in the same routine. (wl_closure_invoke() -> ? -> wl_display_flush_clients())</div><div>    Here, client will be destroyed if wl_connection_flush() returns negative value and errno is not EAGAIN.</div></div></blockquote><div><br></div><div>Out of interest, what is the '?' here? I can't find any examples of this happening in Wayland, Weston, or Mesa. If you could share your usecase, this would be quite interesting. I believe the idea behind deferring the flush was to minimise the number of system calls made, and also somewhat decrease the latency when multiple clients are involved, by batching all the flushes (potentially time-consuming) to a point where we do not have any more requests to process (so are not sensitive to latency).</div><div><br></div><div>In general, Wayland compositors should always be responsive: client request handling should be lightweight and not time-consuming. Wayland clients should also be completely asynchronous, rather than following a RPC/method-return pattern of { send_request(); block_for_reply(); }. Given that, the only reason to flush inside request processing would be because you are sending a lot of data, but again the Wayland protocol itself should not be carrying large amounts of data. So I'm not sure what usecase there is for this.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>3. It will come back to the while loop in wl_client_connection_data().</div><div>    If wl_connection_pending_<wbr>input() returns any values which is bigger than “size of p”; it will go into the while loop again, and then it will do something with the client which is already freed.</div><div><br></div><div><br></div><div>The simplest way to solve this is checking the source(wl_event_source)’ fd, but for now there is no way to access wl_event_source in wayland-server.c. (wl_event_source will be freed a while later)</div></div></blockquote><div><br></div><div>Hm, this is racy though.</div><div><br></div><div>I'm would suggest we document that invoking wl_display_flush_clients() / wl_connection_flush() from a request handler may cause undefined behaviour, and that it must only be called outside of dispatch.</div><div><br></div><div>Cheers,</div><div>Daniel</div></div></div></div>