Re: [Mesa3d-users] [Mesa3d-dev] wglMakeCurrent possible memory leak

[Karl S. <kar...@gm...>]

On Tue, Sep 8, 2009 at 11:19 AM, José Fonseca <jfo...@vm...> wrote:

> On Tue, 2009-09-08 at 09:20 -0700, Karl Schultz wrote:
> >
> >
> > On Tue, Sep 8, 2009 at 9:17 AM, Brian Paul <br...@vm...> wrote:
> >         Dimitar Kodjabachev wrote:
> >         > Hello,
> >         >
> >         > I sent this to mesa3d-dev, which was probably not the right
> >         place.
> >
> >
> >         mesa3d-dev is OK since there may be a bug in the Mesa wgl
> >         code.  I'm
> >         cc'ing mesa3d-dev.
> >
> >
> >         > I have the following situation (pseudocode) with Mesa 7.4.4:
> >         >
> >         > hglrc1 = wglCreateContext(hdc1)
> >         > wglMakeCurrent(hdc1,hglrc1)
> >         > hglrc2 = wglCreateContext(hdc2)
> >         > wglMakeCurrent(hdc2,hglrc2)
> >         > wglMakeCurrent(hdc3,hglrc2)
> >         > wglDeleteContext(hglrc1)
> >         > wglDeleteContext(hglrc2)
> >         >
> >         > As a result of this sequence of calls, a WMesaFramebuffer
> >         structure is leaked.
> >         > The call causing the leak is wglMakeCurrent(hdc3,hglrc2).
> >         This is what I know
> >         > so far:
> >         >
> >         > -> wglMakeCurrent(hdc3,hglrc2) calls
> >         >  -> WMesaMakeCurrent(hglrc2,hdc3) which calls
> >         >  -> wmesa_lookup_framebuffer(hdc3) which returns NULL, as
> >         the hdc given to
> >         >      wglMakeCurrent is different from the one given at the
> >         time
> >         >      of wglCreateContext
> >         >  -> wmesa_new_framebuffer() is called and a new frame buffer
> >         is allocated
> >         >
> >         > This new frame buffer is not released upon
> >         wglDeleteContext(hglrc2).
> >
> >
> >         A rendering context is not the same as a drawing surface.  So
> >         deleting
> >         a rendering context does not imply deleting a surface.
> >
> >
> >         > As far as
> >         > I know, the device context passed to wglMakeCurrent does not
> >         have to
> >         > be the same as the one passed to wglCreateContext as long as
> >         the the actual
> >         > device and the pixel formats are.  Is this correct or is my
> >         usage wrong?
> >
> >
> >         My understanding is that the context passed to
> >         wglMakeCurrent() must
> >         have been created by a call to wglCreateContext().
> >
> >
> > Right, but there are two contexts passed in wglMakeCurrent().  Here
> > the GL context (rendering context) is hglrc2, created by
> > wglCreateContext().  But the Window context (device context) is hdc3,
> > which is just a Windows Win32 handle.
> >
> >
> >         I don't think I can help with this memory leak.  Maybe someone
> >         else
> >         who works on Windows can help you.
> >
> >
> > The problem is that wglMakeCurrent() will lazily allocate the
> > framebuffer when a device context is made current for the first time.
> > The created framebuffer gets added onto a list and is tagged with the
> > device context that it was just associated with.  When a rendering
> > context is destroyed, this list is searched for a framebuffer tagged
> > with the device context that is current at the time the rendering
> > context was destroyed, and if found, the framebuffer is freed.
> >
> > Note that this conflicts with what Brian said above about deleting
> > surfaces.  I imagine that this was added to wgl as a way to free the
> > framebuffer *somehow*.
> >
> > The leak is caused here because the rendering context is associated
> > with two device contexts.  A framebuffer is created for each device.
> > Later, when the rendering context is destroyed, only one framebuffer
> > is freed, because only one device context can be current when
> > destroying the rendering context, and only the framebuffer tagged with
> > that device context is freed.
> >
> > Since destroying a rendering context shouldn't destroy any surfaces,
> > it isn't an option to tag all framebuffers with the rendering context
> > id and then delete all framebuffers tagged that way when a rendering
> > context is destroyed.  Although this would fix the leak.
> >
> > The right way, I think, is to destroy the framebuffer associated with
> > a device context when the device context is destroyed.  This might
> > involve hooking Windows somehow to get a notification when it is
> > destroyed or setting up a message handler to get WM_DESTROY messages
> > so that we can free resources associated with the device.
> >
> > The wgl code hasn't been all that robust or complete, and I don't know
> > right now if this design/approach is even correct.  I'd have to dig
> > into this more.
> >
> > Right now, I don't have much of a workaround to suggest for the
> > current implementation except to try to destroy hglrc2 and recreate it
> > before making current with hdc3.
> >
> >
> > Karl
>
> What WGL implementation are we talking about here? One of the pure Mesa
> based one in src/mesa/drivers/windows/gdi, or the Gallium based one in
> src/gallium/state_trackers/wgl ?
>
>
I was talking about the former - the pure Mesa one.  I think that the OP was
talking about the same.


> The Gallium WGL state tracker is a faily complete implementation of both
> WGL and ICD interfaces and at least the ICD interface has received a lot
> of testing. The drawback is that the only available driver for it ATM is
> softpipe, which is in generally slower than Mesa's swrast.
>
>
Yes, that implementation of wgl is much better.  It does have a windowproc
to handle resize and destroy events, which frees the framebuffer resources
as I had suggested.


> Jose
>
>