On Tue, 2 Oct 2012 21:12:35 -0700 "U. Artie Eoff" <[email protected]> wrote:
> From: "U. Artie Eoff" <[email protected]> > > desktop-shell never returned from display_run() since it > was essentially killed when weston exited. To fix this, > it is necessary to watch for EPOLLHUP in window.c so that > toytoolkit clients will return from display_run() when > weston quits. This allows for clients to clean up > as needed. > > Signed-off-by: U. Artie Eoff <[email protected]> Hi Artie, I very much like this, though I didn't carefully review the patch. This also answers Scott's questions about where things are freed. ;-) One minor suggestion at the end. > --- > clients/desktop-shell.c | 88 > +++++++++++++++++++++++++++++++++++++++++++++++++ > clients/window.c | 21 +++++++++--- > 2 files changed, 105 insertions(+), 4 deletions(-) > <...> > diff --git a/clients/window.c b/clients/window.c > index f3b61de..6695978 100644 > --- a/clients/window.c > +++ b/clients/window.c > @@ -84,6 +84,7 @@ struct display { > uint32_t serial; > > int display_fd; > + uint32_t display_fd_events; > uint32_t mask; > struct task display_task; > > @@ -3727,7 +3728,14 @@ handle_display_data(struct task *task, uint32_t events) > { > struct display *display = > container_of(task, struct display, display_task); > - > + > + display->display_fd_events = events; > + > + if (events & EPOLLERR || events & EPOLLHUP) { > + display_exit(display); > + return; > + } > + > wl_display_iterate(display->display, display->mask); > } > > @@ -3751,7 +3759,8 @@ display_create(int argc, char *argv[]) > d->epoll_fd = os_epoll_create_cloexec(); > d->display_fd = wl_display_get_fd(d->display, event_mask_update, d); > d->display_task.run = handle_display_data; > - display_watch_fd(d, d->display_fd, EPOLLIN, &d->display_task); > + display_watch_fd(d, d->display_fd, EPOLLIN | EPOLLERR | EPOLLHUP, > + &d->display_task); > > wl_list_init(&d->deferred_list); > wl_list_init(&d->input_list); > @@ -3815,7 +3824,8 @@ void > display_destroy(struct display *display) > { > if (!wl_list_empty(&display->window_list)) > - fprintf(stderr, "toytoolkit warning: windows exist.\n"); > + fprintf(stderr, "toytoolkit warning: %d windows exist.\n", > + wl_list_length(&display->window_list)); > > if (!wl_list_empty(&display->deferred_list)) > fprintf(stderr, "toytoolkit warning: deferred tasks exist.\n"); > @@ -3845,7 +3855,10 @@ display_destroy(struct display *display) > > close(display->epoll_fd); > > - wl_display_flush(display->display); > + if (!(display->display_fd_events & EPOLLERR) && > + !(display->display_fd_events & EPOLLHUP)) > + wl_display_flush(display->display); As display_exit() simply sets display->running to zero, maybe you could replace display->running with something that can indicate "running", "exiting", and "exiting with Wayland connection error", the last one only triggered by EPOLLERR and EPOLLHUP. It would make this last if-statement more obvious. > + > wl_display_disconnect(display->display); > free(display); > } Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
