<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Giulio,<div class=""><br class=""></div><div class="">Sorry for resending same email. (previous one got failed to send to <a href="mailto:wayland-devel@lists.freedesktop.org" class="">wayland-devel@lists.freedesktop.org</a>; so I’m sending the same one again)</div><div class=""><br class=""></div><div class="">in wl_client_connection_data(),</div><div class="">there is no guarantee that client would never be destroyed in the process of handling the pending input.</div><div class=""><br class=""></div><div class="">For example,</div><div class="">1. It comes to wl_client_connection_data()</div><div class="">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 class=""> Here, client will be destroyed if wl_connection_flush() returns negative value and errno is not EAGAIN.</div><div class="">3. It will come back to the while loop in wl_client_connection_data().</div><div class=""> If wl_connection_pending_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 class=""><br class=""></div><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">Thanks,</div><div class=""><br class=""></div><div class=""> <div style="color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><span style="font-size: 13px;" class="">Hyunkook Khang / 강 현 국</span><br style="font-size: 13px;" class=""><span style="font-size: 13px;" class="">Research Engineer</span><br style="font-size: 13px;" class=""><span style="font-size: 13px;" class="">Reference Platform Task</span><br style="font-size: 13px;" class=""><span style="font-size: 13px;" class="">Mobile: 82-10-7249-9909</span><br style="font-size: 13px;" class=""><span style="font-size: 13px;" class="">E-mail: </span><a href="mailto:hyunkook.khang@lge.com" class=""><font size="2" class="">h</font>yunkook.khang@lge.com</a></div><div class=""><span style="font-size: 13px;" class="">Seocho R&D Campus, 19, Yangjae-daero 11gil</span><br style="font-size: 13px;" class=""><span style="font-size: 13px;" class="">Seocho-gu, Seoul 137-130, Korea</span><br style="font-size: 13px;" class=""><br style="font-size: 13px;" class=""></div></div><span style="color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class="Apple-interchange-newline" style="color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span style="color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span><img height="235" width="602" apple-inline="yes" id="429D635E-D80F-407D-A5E7-A81F804CA8C4" apple-width="yes" apple-height="yes" src="cid:19E2FB44-5F4B-48B7-BBDB-28C8ED70EFA2" class=""></span> </span></span></div> <br class=""><div><blockquote type="cite" class=""><div class="">On Sep 22, 2016, at 4:50 PM, Giulio Camuffo <<a href="mailto:giuliocamuffo@gmail.com" class="">giuliocamuffo@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hi,<br class=""><br class="">could you write how to trigger the crash, also in the commit message<br class="">maybe? Besides that, i have a comment inline below.<br class=""><br class="">2016-09-21 9:08 GMT+02:00 Hyunkook Khang <<a href="mailto:hyunkook.khang@lge.com" class="">hyunkook.khang@lge.com</a>>:<br class=""><blockquote type="cite" class="">While processing pending data, client could be destroyed in the middle of<br class="">the process. (e.g. by invoking wl_display_flush_clients()).<br class="">In this case, client will be freed, but we are still in the processing data<br class="">of the client, so it could cause a crash.<br class=""><br class="">To avoid this, instead of destroying the client directly,<br class="">just set the error here and destroy the client where it needs to be.<br class=""><br class="">Signed-off-by: Hyunkook Khang <<a href="mailto:hyunkook.khang@lge.com" class="">hyunkook.khang@lge.com</a>><br class="">---<br class=""> src/wayland-server.c | 8 +++++++-<br class=""> 1 file changed, 7 insertions(+), 1 deletion(-)<br class=""><br class="">diff --git a/src/wayland-server.c b/src/wayland-server.c<br class="">index 9d7d9c1..89d0bac 100644<br class="">--- a/src/wayland-server.c<br class="">+++ b/src/wayland-server.c<br class="">@@ -1103,10 +1103,16 @@ wl_display_terminate(struct wl_display *display)<br class=""> WL_EXPORT void<br class=""> wl_display_run(struct wl_display *display)<br class=""> {<br class="">+ struct wl_client *client, *next;<br class="">+<br class=""> display->run = 1;<br class=""><br class=""> while (display->run) {<br class=""> wl_display_flush_clients(display);<br class="">+ wl_list_for_each_safe(client, next, &display->client_list, link) {<br class="">+ if (client->error)<br class="">+ wl_client_destroy(client);<br class="">+ }<br class=""></blockquote><br class="">wl_display_flush_clients() may be called manually by the user too,<br class="">without using wl_display_run(). So now all the users would need to do<br class="">the loop to destroy the clients, or else they would leak. Instead, you<br class="">should do the loop at the end of wl_display_flush_clients().<br class=""><br class=""><br class="">Thanks,<br class="">Giulio<br class=""><br class=""><blockquote type="cite" class=""> wl_event_loop_dispatch(display->loop, -1);<br class=""> }<br class=""> }<br class="">@@ -1124,7 +1130,7 @@ wl_display_flush_clients(struct wl_display *display)<br class=""> WL_EVENT_WRITABLE |<br class=""> WL_EVENT_READABLE);<br class=""> } else if (ret < 0) {<br class="">- wl_client_destroy(client);<br class="">+ client->error = 1;<br class=""> }<br class=""> }<br class=""> }<br class="">--<br class="">1.7.9.5<br class=""><br class="">_______________________________________________<br class="">wayland-devel mailing list<br class=""><a href="mailto:wayland-devel@lists.freedesktop.org" class="">wayland-devel@lists.freedesktop.org</a><br class="">https://lists.freedesktop.org/mailman/listinfo/wayland-devel<br class=""></blockquote>_______________________________________________<br class="">wayland-devel mailing list<br class=""><a href="mailto:wayland-devel@lists.freedesktop.org" class="">wayland-devel@lists.freedesktop.org</a><br class="">https://lists.freedesktop.org/mailman/listinfo/wayland-devel<br class=""></div></div></blockquote></div><br class=""></body></html>