[Spice-devel] [PATCH spice-gtk 6/7] egl: don't terminate display

[Victor Toso]

Hi,

On Mon, May 23, 2016 at 06:39:25AM -0400, Marc-André Lureau wrote:
> 
> 
> ----- Original Message -----
> > Hi,
> > 
> > On Mon, May 23, 2016 at 05:53:35AM -0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > ----- Original Message -----
> > > > Hi,
> > > >
> > > > On Mon, May 23, 2016 at 11:31:52AM +0200, Marc-André Lureau wrote:
> > > > > Hi
> > > > >
> > > > > On Mon, May 23, 2016 at 10:21 AM, Victor Toso <lists at victortoso.com>
> > > > > wrote:
> > > > > > Could you also include a fix that calls eglTerminate ?
> > > > >
> > > > > Since this is global resources, spice-gtk (a library) can't do that by
> > > > > itself without risking to break somebody else code.
> > > >
> > > > Sorry, I don't really follow. The spice-gtk lib does eglInitialize on
> > > > spice_egl_init but we should not eglTerminate at some point?
> > >
> > > Yes, " Initializing an already initialized EGL display connection has
> > > no effect besides returning the version numbers. "
> > 
> > Right.
> > 
> > > this is not the case with eglterminate, we can't call it as egl may be
> > > used by somebody else code.
> > 
> > So far I understand that we might be calling eglTerminate when we still
> > will use it the EGLDisplay which I thought we could eglTerminate when
> > session ends, for instance.
> > 
> > If spice-gtk does eglInitialize (not the application) I would like to
> > understaund in this case why can't eglTerminate. Is it because the egl
> > API does not inform that spice_egl_init was the one that initialized the
> > EGLDisplay ?
>
> Not only eglinitialize() doesn't say if it's already initialized, but
> somebody else code could call eglinitialize() later too.
>
> > Either way, this really needs a comment in the unrealize function
> > instead of just removing the function call. We will see leaks from this.
>
> Not really a leak, since it's a one time leak, sort of like a static/global.
>
> I'll put a comment there:
> /* do not call eglterminate() since egl may be used by
>          * somebody else code */

Thanks for the explanation,
Acked-by: Victor Toso <victortoso at redhat.com>

>
> > >
> > > >
> > > > >
> > > > > --
> > > > > Marc-André Lureau
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> >