Hi, I checked e-mail before my leave to vacation and saw this message, so here's a quick response:
The two questions answered Pekka (thanks). More discussion about this pthread_cond broadcasting is here: http://lists.freedesktop.org/archives/wayland-devel/2014-June/015632.html http://lists.freedesktop.org/archives/wayland-devel/2014-July/016045.html I've recently was looking into it and have some patches locally. I think there's a bug and the broadcast should be there, but for display->reader_cond. For this condition is waited in read_events. When a thread is waiting for an access to the display's fd, it would block after an error without this broadcast. I have some work in progress regarding this: https://github.com/mchalupa/wayland/commits/threading It's still in hard development, but seems working. I haven't got to some extensive testing yet, though. See you in two weeks :) Regards, Marek On 23 July 2014 04:44, Jason Ekstrand <[email protected]> wrote: > Sorry for not looking at it. I'm mostly ok with the API so it's fine that > you pushed it. I did have a question or two though. > > > On Mon, Jul 7, 2014 at 7:28 AM, Pekka Paalanen <[email protected]> > wrote: > >> From: Marek Chalupa <[email protected]> >> >> When an error occurs, wl_display_get_error() does not >> provide any way of getting know if it was a local error or if it was >> an error event, respectively what object caused the error and what >> the error was. >> >> This patch introduces a new function wl_display_get_protocol_error() >> which will return error code, interface and id of the object that >> generated the error. >> wl_display_get_error() will work the same way as before. >> >> wl_display_get_protocol_error() DOES NOT indicate that a non-protocol >> error happened. It returns valid information only in that case that >> (protocol) error occurred, so it should be used after calling >> wl_display_get_error() with positive result. >> >> [Pekka Paalanen] Applied another hunk of Bryce's comments to docs, >> added libtool version bump. >> >> Reviewed-by: Pekka Paalanen <[email protected]> >> Reviewed-by: Bryce Harrington <[email protected]> >> --- >> Makefile.am | 2 +- >> src/wayland-client.c | 137 >> ++++++++++++++++++++++++++++++++++++++++++++------- >> src/wayland-client.h | 3 ++ >> 3 files changed, 123 insertions(+), 19 deletions(-) >> >> diff --git a/Makefile.am b/Makefile.am >> index c15d8b8..fee19ab 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -51,7 +51,7 @@ nodist_libwayland_server_la_SOURCES = \ >> >> libwayland_client_la_CFLAGS = $(FFI_CFLAGS) $(GCC_CFLAGS) -pthread >> libwayland_client_la_LIBADD = $(FFI_LIBS) libwayland-util.la -lrt -lm >> -libwayland_client_la_LDFLAGS = -version-info 2:0:2 >> +libwayland_client_la_LDFLAGS = -version-info 3:0:3 >> libwayland_client_la_SOURCES = \ >> src/wayland-client.c >> >> diff --git a/src/wayland-client.c b/src/wayland-client.c >> index e8aab7e..19b5694 100644 >> --- a/src/wayland-client.c >> +++ b/src/wayland-client.c >> @@ -78,7 +78,24 @@ struct wl_event_queue { >> struct wl_display { >> struct wl_proxy proxy; >> struct wl_connection *connection; >> + >> + /* errno of the last wl_display error */ >> int last_error; >> + >> + /* When display gets an error event from some object, it stores >> + * information about it here, so that client can get this >> + * information afterwards */ >> + struct { >> + /* Code of the error. It can be compared to >> + * the interface's errors enumeration. */ >> + uint32_t code; >> + /* interface (protocol) in which the error occurred */ >> + const struct wl_interface *interface; >> + /* id of the proxy that caused the error. There's no >> warranty >> + * that the proxy is still valid. It's up to client how >> it will >> + * use it */ >> + uint32_t id; >> + } protocol_error; >> int fd; >> pthread_t display_thread; >> struct wl_map objects; >> @@ -96,6 +113,14 @@ struct wl_display { >> >> static int debug_client = 0; >> >> +/** >> + * This function is called for local errors (no memory, server hung up) >> + * >> + * \param display >> + * \param error error value (EINVAL, EFAULT, ...) >> + * >> + * \note this function is called with display mutex locked >> + */ >> static void >> display_fatal_error(struct wl_display *display, int error) >> { >> @@ -105,7 +130,7 @@ display_fatal_error(struct wl_display *display, int >> error) >> return; >> >> if (!error) >> - error = 1; >> + error = EFAULT; >> >> display->last_error = error; >> >> @@ -113,11 +138,56 @@ display_fatal_error(struct wl_display *display, int >> error) >> pthread_cond_broadcast(&iter->cond); >> } >> >> +/** >> + * This function is called for error events >> + * and indicates that in some object an error occured. >> + * Difference between this function and display_fatal_error() >> + * is that this one handles errors that will come by wire, >> + * whereas display_fatal_error() is called for local errors. >> + * >> + * \param display >> + * \param code error code >> + * \param id id of the object that generated the error >> + * \param intf protocol interface >> + */ >> static void >> -wl_display_fatal_error(struct wl_display *display, int error) >> +display_protocol_error(struct wl_display *display, uint32_t code, >> + uint32_t id, const struct wl_interface *intf) >> { >> + struct wl_event_queue *iter; >> + int err; >> + >> + if (display->last_error) >> + return; >> + >> + /* set correct errno */ >> + if (wl_interface_equal(intf, &wl_display_interface)) { >> + switch (code) { >> + case WL_DISPLAY_ERROR_INVALID_OBJECT: >> + case WL_DISPLAY_ERROR_INVALID_METHOD: >> + err = EINVAL; >> + break; >> + case WL_DISPLAY_ERROR_NO_MEMORY: >> + err = ENOMEM; >> + break; >> + default: >> + err = EFAULT; >> + } >> + } else { >> + err = EPROTO; >> + } >> + >> pthread_mutex_lock(&display->mutex); >> - display_fatal_error(display, error); >> + >> + display->last_error = err; >> + >> + display->protocol_error.code = code; >> + display->protocol_error.id = id; >> + display->protocol_error.interface = intf; >> + >> + wl_list_for_each(iter, &display->event_queue_list, link) >> + pthread_cond_broadcast(&iter->cond); >> > > Why are you doing this? Is someone waiting on you to display an error or > waiting for the error code? I don't really see what there is to wake up > here. > > >> + >> pthread_mutex_unlock(&display->mutex); >> } >> >> @@ -579,25 +649,12 @@ display_handle_error(void *data, >> uint32_t code, const char *message) >> { >> struct wl_proxy *proxy = object; >> - int err; >> >> wl_log("%s@%u: error %d: %s\n", >> proxy->object.interface->name, proxy->object.id, code, >> message); >> >> - switch (code) { >> - case WL_DISPLAY_ERROR_INVALID_OBJECT: >> - case WL_DISPLAY_ERROR_INVALID_METHOD: >> - err = EINVAL; >> - break; >> - case WL_DISPLAY_ERROR_NO_MEMORY: >> - err = ENOMEM; >> - break; >> - default: >> - err = EFAULT; >> - break; >> - } >> - >> - wl_display_fatal_error(display, err); >> + display_protocol_error(display, code, proxy->object.id, >> + proxy->object.interface); >> } >> >> static void >> @@ -1486,6 +1543,50 @@ wl_display_get_error(struct wl_display *display) >> return ret; >> } >> >> +/** >> + * Retrieves the information about a protocol error: >> + * >> + * \param display The Wayland display >> + * \param interface if not NULL, stores the interface where the error >> occurred >> + * \param id if not NULL, stores the object id that generated >> + * the error. There's no guarantee the object is >> + * still valid; the client must know if it deleted the >> object. >> > > Does the client even know what to do with the ID? I mean, sure, they > could print it in an error message, but how useful is it? I suppose they > could check to see if wl_proxy_get_id(my_proxy) == id to see if it happend > at that point in the stream. Is this what you intend? > > >> + * \return The error code as defined in the interface >> specification. >> + * >> + * \code >> + * int err = wl_display_get_error(display); >> + * >> + * if (err == EPROTO) { >> + * code = wl_display_get_protocol_error(display, &interface, &id); >> + * handle_error(code, interface, id); >> + * } >> + * >> + * ... >> + * >> + * \endcode >> + */ >> +WL_EXPORT uint32_t >> +wl_display_get_protocol_error(struct wl_display *display, >> + const struct wl_interface **interface, >> + uint32_t *id) >> +{ >> + uint32_t ret; >> + >> + pthread_mutex_lock(&display->mutex); >> + >> + ret = display->protocol_error.code; >> + >> + if (interface) >> + *interface = display->protocol_error.interface; >> + if (id) >> + *id = display->protocol_error.id; >> + >> + pthread_mutex_unlock(&display->mutex); >> + >> + return ret; >> +} >> + >> + >> /** Send all buffered requests on the display to the server >> * >> * \param display The display context object >> diff --git a/src/wayland-client.h b/src/wayland-client.h >> index 2a32785..dc4df53 100644 >> --- a/src/wayland-client.h >> +++ b/src/wayland-client.h >> @@ -161,6 +161,9 @@ int wl_display_dispatch_queue_pending(struct >> wl_display *display, >> struct wl_event_queue *queue); >> int wl_display_dispatch_pending(struct wl_display *display); >> int wl_display_get_error(struct wl_display *display); >> +uint32_t wl_display_get_protocol_error(struct wl_display *display, >> + const struct wl_interface >> **interface, >> + uint32_t *id); >> >> int wl_display_flush(struct wl_display *display); >> int wl_display_roundtrip(struct wl_display *display); >> -- >> 1.8.5.5 >> >> _______________________________________________ >> wayland-devel mailing list >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel >> > >
_______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
