[RFC,v2,0/8] libcamera: software_isp: debayer_egl: Remove some leaks
mbox series

Message ID 20260810105942.1098192-1-barnabas.pocze@ideasonboard.com
Headers show
Series
  • libcamera: software_isp: debayer_egl: Remove some leaks
Related show

Message

Barnabás Pőcze Aug. 10, 2026, 10:59 a.m. UTC 
Currently `eGL::initEGLContext()` is called each time the camera is started,
so the `EGLContext` is leaked if the camera is started more than once.

Furthermore, mesa does not seem to have any thread destructors to clean up
the bound context when the thread is stopped.

So these are some RFC changes intended to address the above leaks.
(As well some other small changes.)

The effects can be observed with asan and e.g.

  $ lc-compliance -f '*CaptureStartStop/StillCapture__8' -c '\_SB_.PC00.LNK1'
  (which does 3 start-stop sequences)

but I have also extended `cam` to be able to repeat the capture sessions, and that
very clearly shows a start-stop leak:

  before:
    repeat | result
         4 | SUMMARY: AddressSanitizer: 8979617 byte(s) leaked in 72738 allocation(s).
         8 | SUMMARY: AddressSanitizer: 17536873 byte(s) leaked in 144057 allocation(s).
        16 | SUMMARY: AddressSanitizer: 34717529 byte(s) leaked in 286697 allocation(s).
        32 | SUMMARY: AddressSanitizer: 69078457 byte(s) leaked in 571977 allocation(s).

  after:
    repeat | result
        32 | SUMMARY: AddressSanitizer: 487117 byte(s) leaked in 1425 allocation(s).

changes in v2:
  * drop the explicit context un-make and move it into `resetEGLContext()`
  * assert the appropriate thread after the removal of the explicit make-context call

v1: https://patchwork.libcamera.org/cover/27602/

Barnabás Pőcze (8):
  libcamera: egl: Remove `gl{Use,Delete}Program()`
  libcamera: egl: Remove `eGL::surface_`
  libcamera: egl: Do not load `glGetString` dynamically
  libcamera: egl: Ensure all members are always initialized
  libcamera: egl: Add `resetEGLContext()`
  libcamera: software_isp: debayer_egl: Avoid EGL context leaks
  libcamera: egl: initEGLContext(): Avoid double init
  libcamera: software_isp: debayer_egl: Remove EGL context switch

 include/libcamera/internal/egl.h           |  24 +++--
 src/libcamera/egl.cpp                      | 102 +++++++++------------
 src/libcamera/software_isp/debayer_egl.cpp |   8 +-
 3 files changed, 63 insertions(+), 71 deletions(-)

--
2.55.0

Comments

Jacopo Mondi Aug. 11, 2026, 7:49 a.m. UTC | #1 
Hi Barnabás

On Mon, Aug 10, 2026 at 12:59:34PM +0200, Barnabás Pőcze wrote:
> Currently `eGL::initEGLContext()` is called each time the camera is started,
> so the `EGLContext` is leaked if the camera is started more than once.
>
> Furthermore, mesa does not seem to have any thread destructors to clean up
> the bound context when the thread is stopped.
>
> So these are some RFC changes intended to address the above leaks.

Is this series something you want to merge or is it an RFC just to
collect comments ?

> (As well some other small changes.)
>
> The effects can be observed with asan and e.g.
>
>   $ lc-compliance -f '*CaptureStartStop/StillCapture__8' -c '\_SB_.PC00.LNK1'
>   (which does 3 start-stop sequences)
>
> but I have also extended `cam` to be able to repeat the capture sessions, and that
> very clearly shows a start-stop leak:
>
>   before:
>     repeat | result
>          4 | SUMMARY: AddressSanitizer: 8979617 byte(s) leaked in 72738 allocation(s).
>          8 | SUMMARY: AddressSanitizer: 17536873 byte(s) leaked in 144057 allocation(s).
>         16 | SUMMARY: AddressSanitizer: 34717529 byte(s) leaked in 286697 allocation(s).
>         32 | SUMMARY: AddressSanitizer: 69078457 byte(s) leaked in 571977 allocation(s).
>
>   after:
>     repeat | result
>         32 | SUMMARY: AddressSanitizer: 487117 byte(s) leaked in 1425 allocation(s).
>
> changes in v2:
>   * drop the explicit context un-make and move it into `resetEGLContext()`
>   * assert the appropriate thread after the removal of the explicit make-context call
>
> v1: https://patchwork.libcamera.org/cover/27602/
>
> Barnabás Pőcze (8):
>   libcamera: egl: Remove `gl{Use,Delete}Program()`
>   libcamera: egl: Remove `eGL::surface_`
>   libcamera: egl: Do not load `glGetString` dynamically
>   libcamera: egl: Ensure all members are always initialized
>   libcamera: egl: Add `resetEGLContext()`
>   libcamera: software_isp: debayer_egl: Avoid EGL context leaks
>   libcamera: egl: initEGLContext(): Avoid double init
>   libcamera: software_isp: debayer_egl: Remove EGL context switch
>
>  include/libcamera/internal/egl.h           |  24 +++--
>  src/libcamera/egl.cpp                      | 102 +++++++++------------
>  src/libcamera/software_isp/debayer_egl.cpp |   8 +-
>  3 files changed, 63 insertions(+), 71 deletions(-)
>
> --
> 2.55.0
Barnabás Pőcze Aug. 11, 2026, 7:56 a.m. UTC | #2 
2026. 08. 11. 9:49 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Mon, Aug 10, 2026 at 12:59:34PM +0200, Barnabás Pőcze wrote:
>> Currently `eGL::initEGLContext()` is called each time the camera is started,
>> so the `EGLContext` is leaked if the camera is started more than once.
>>
>> Furthermore, mesa does not seem to have any thread destructors to clean up
>> the bound context when the thread is stopped.
>>
>> So these are some RFC changes intended to address the above leaks.
> 
> Is this series something you want to merge or is it an RFC just to
> collect comments ?

I would just like to see the issues fixed. If someone has a better approach,
I have no issues abandoning this. But if that's not the case and these changes
seem acceptable, then yes, I would like these to be merged.



> 
>> (As well some other small changes.)
>>
>> The effects can be observed with asan and e.g.
>>
>>    $ lc-compliance -f '*CaptureStartStop/StillCapture__8' -c '\_SB_.PC00.LNK1'
>>    (which does 3 start-stop sequences)
>>
>> but I have also extended `cam` to be able to repeat the capture sessions, and that
>> very clearly shows a start-stop leak:
>>
>>    before:
>>      repeat | result
>>           4 | SUMMARY: AddressSanitizer: 8979617 byte(s) leaked in 72738 allocation(s).
>>           8 | SUMMARY: AddressSanitizer: 17536873 byte(s) leaked in 144057 allocation(s).
>>          16 | SUMMARY: AddressSanitizer: 34717529 byte(s) leaked in 286697 allocation(s).
>>          32 | SUMMARY: AddressSanitizer: 69078457 byte(s) leaked in 571977 allocation(s).
>>
>>    after:
>>      repeat | result
>>          32 | SUMMARY: AddressSanitizer: 487117 byte(s) leaked in 1425 allocation(s).
>>
>> changes in v2:
>>    * drop the explicit context un-make and move it into `resetEGLContext()`
>>    * assert the appropriate thread after the removal of the explicit make-context call
>>
>> v1: https://patchwork.libcamera.org/cover/27602/
>>
>> Barnabás Pőcze (8):
>>    libcamera: egl: Remove `gl{Use,Delete}Program()`
>>    libcamera: egl: Remove `eGL::surface_`
>>    libcamera: egl: Do not load `glGetString` dynamically
>>    libcamera: egl: Ensure all members are always initialized
>>    libcamera: egl: Add `resetEGLContext()`
>>    libcamera: software_isp: debayer_egl: Avoid EGL context leaks
>>    libcamera: egl: initEGLContext(): Avoid double init
>>    libcamera: software_isp: debayer_egl: Remove EGL context switch
>>
>>   include/libcamera/internal/egl.h           |  24 +++--
>>   src/libcamera/egl.cpp                      | 102 +++++++++------------
>>   src/libcamera/software_isp/debayer_egl.cpp |   8 +-
>>   3 files changed, 63 insertions(+), 71 deletions(-)
>>
>> --
>> 2.55.0
Jacopo Mondi Aug. 11, 2026, 9:07 a.m. UTC | #3 
Hi Barnabás

On Tue, Aug 11, 2026 at 09:56:54AM +0200, Barnabás Pőcze wrote:
> 2026. 08. 11. 9:49 keltezéssel, Jacopo Mondi írta:
> > Hi Barnabás
> >
> > On Mon, Aug 10, 2026 at 12:59:34PM +0200, Barnabás Pőcze wrote:
> > > Currently `eGL::initEGLContext()` is called each time the camera is started,
> > > so the `EGLContext` is leaked if the camera is started more than once.
> > >
> > > Furthermore, mesa does not seem to have any thread destructors to clean up
> > > the bound context when the thread is stopped.
> > >
> > > So these are some RFC changes intended to address the above leaks.
> >
> > Is this series something you want to merge or is it an RFC just to
> > collect comments ?
>
> I would just like to see the issues fixed. If someone has a better approach,
> I have no issues abandoning this. But if that's not the case and these changes

I'm not sure I get you here. Do you know about someone else working on
this ?

> seem acceptable, then yes, I would like these to be merged.

So why an RFC ? RFC means "I don't want this merged, just let me know
what you think".


>
>
>
> >
> > > (As well some other small changes.)
> > >
> > > The effects can be observed with asan and e.g.
> > >
> > >    $ lc-compliance -f '*CaptureStartStop/StillCapture__8' -c '\_SB_.PC00.LNK1'
> > >    (which does 3 start-stop sequences)
> > >
> > > but I have also extended `cam` to be able to repeat the capture sessions, and that
> > > very clearly shows a start-stop leak:
> > >
> > >    before:
> > >      repeat | result
> > >           4 | SUMMARY: AddressSanitizer: 8979617 byte(s) leaked in 72738 allocation(s).
> > >           8 | SUMMARY: AddressSanitizer: 17536873 byte(s) leaked in 144057 allocation(s).
> > >          16 | SUMMARY: AddressSanitizer: 34717529 byte(s) leaked in 286697 allocation(s).
> > >          32 | SUMMARY: AddressSanitizer: 69078457 byte(s) leaked in 571977 allocation(s).
> > >
> > >    after:
> > >      repeat | result
> > >          32 | SUMMARY: AddressSanitizer: 487117 byte(s) leaked in 1425 allocation(s).
> > >
> > > changes in v2:
> > >    * drop the explicit context un-make and move it into `resetEGLContext()`
> > >    * assert the appropriate thread after the removal of the explicit make-context call
> > >
> > > v1: https://patchwork.libcamera.org/cover/27602/
> > >
> > > Barnabás Pőcze (8):
> > >    libcamera: egl: Remove `gl{Use,Delete}Program()`
> > >    libcamera: egl: Remove `eGL::surface_`
> > >    libcamera: egl: Do not load `glGetString` dynamically
> > >    libcamera: egl: Ensure all members are always initialized
> > >    libcamera: egl: Add `resetEGLContext()`
> > >    libcamera: software_isp: debayer_egl: Avoid EGL context leaks
> > >    libcamera: egl: initEGLContext(): Avoid double init
> > >    libcamera: software_isp: debayer_egl: Remove EGL context switch
> > >
> > >   include/libcamera/internal/egl.h           |  24 +++--
> > >   src/libcamera/egl.cpp                      | 102 +++++++++------------
> > >   src/libcamera/software_isp/debayer_egl.cpp |   8 +-
> > >   3 files changed, 63 insertions(+), 71 deletions(-)
> > >
> > > --
> > > 2.55.0
>