[PATCH wayland] client: prevent using and creating proxies after an error occured

[Marek Chalupa]

On 22 August 2014 12:56, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Fri, 18 Jul 2014 17:56:36 +0200
> Marek Chalupa <mchqwerty at gmail.com> wrote:
>
> > We won't receive any response anyway...
> >
> > This removes the last line of:
> >
> > [2230782.435] wl_display at 1.error(wl_display at 1, 2, "no memory")
> > wl_display at 1: error 2: no memory
> > [2230782.534]  -> wl_display at 1.sync(new id wl_callback at 3)
> >
> > and prevents from creating proxy on invalidated display object.
> > ---
> >  src/wayland-client.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/wayland-client.c b/src/wayland-client.c
> > index ee2215d..2f52900 100644
> > --- a/src/wayland-client.c
> > +++ b/src/wayland-client.c
> > @@ -312,6 +312,9 @@ wl_proxy_create(struct wl_proxy *factory, const
> struct wl_interface *interface)
> >       struct wl_display *display = factory->display;
> >       struct wl_proxy *proxy;
> >
> > +     if (display->last_error)
> > +             return NULL;
> > +
> >       pthread_mutex_lock(&display->mutex);
> >       proxy = proxy_create(factory, interface);
> >       pthread_mutex_unlock(&display->mutex);
> > @@ -523,6 +526,9 @@ wl_proxy_marshal_array_constructor(struct wl_proxy
> *proxy,
> >       struct wl_proxy *new_proxy = NULL;
> >       const struct wl_message *message;
> >
> > +     if (proxy->display->last_error)
> > +             return NULL;
> > +
> >       pthread_mutex_lock(&proxy->display->mutex);
> >
> >       message = &proxy->object.interface->methods[opcode];
>
> Hi,
>
> I think last_error needs to be checked under the display mutex.
>

Yes


> I'm also not so sure about this whole change, because I believe apps
> expect the wl_proxy creation to succeed, and assume that if it fails,
> it is due to memory allocation failure. The reponse to memory
> allocation failure may be just abort(), while the response to a
> protocol error could include saving current work to a temporary file.
>
> AFAICS, the only way to get NULL right now is a memory allocation
> error, and I think that things like glib just abort if malloc ever
> fails, so it is conceivable Gtk and apps will do that too. Whether they
> actually handle protocol errors differently, I don't know, but at least
> a protocol error is much more recoverable than a malloc failure.
>
> Creating more proxies after the wl_display is already disconnected
> should not hurt, right?
>

> I need a second opinion before taking this in. Am I making any sense?
>

Yes, I understand, it makes sense. Let's discard this patch,
since there are no troubles with the proxies now. If one day there are,
we can start solving it again.


>
>
> Thanks,
> pq
>

Thanks,
Marek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140822/c39216ee/attachment.html>