Memory Leak

[danielpaulus]

Hey there, I created the minimal repro case for my memory leak. For some reason memory seems to grow indefinitely when I create a few contexts over the applications lifecycle in combination with listing devices.

func repro() {
    for {
        ctx := gousb.NewContext()
        devs, _ := ctx.OpenDevices(func(desc *gousb.DeviceDesc) bool {
            return true
        })
        for _, d := range devs {
            d.Close()
        }

        ctx.Close()

    }
}

[Gustavomurta]

Hi, for what? I didn't understand your objective. Wrong way, I think. Suggested reading: https://github.com/google/gousb/blob/master/lsusb/main.go

[danielpaulus]

Hey @Gustavomurta this code is just an example for reproducing the memoryleak faster that I am seeing in my program. The code itself should be correct though and although it is not useful for anything other than reproducing the memory leak, it should work just fine. If you run this though you would see that the go process keeps using more memory while the golang heap does not increase.

[danielpaulus]

I am investigating it a little more. If I run the "leaking" func then I can see memory blowing up quickly. If I use the underlying devicelist code then everything seems normal.

package gousb

import (
    "log"
    "os"
    "os/signal"
    "syscall"
)

func TestMemLeak() {
    for i := 1; i <= 20; i++ {
        go leaking()
    }
    c := make(chan os.Signal, 1)
    signal.Notify(c, syscall.SIGINT, syscall.SIGTERM)
    <-c
}
func leaking() {
    for {
        ctx := NewContext()
        devs, _ := ctx.OpenDevices(func(desc *DeviceDesc) bool {
            return true
        })
        for _, d := range devs {
            d.Close()
        }
        ctx.Close()
        //PrintMemUsage()
    }
}

func notLeaking() {
    for {
        c := NewContext()

        list, err := c.libusb.getDevices(c.ctx)
        if err != nil {
            log.Print(err)
        }

        for _, dev := range list {

            desc, err := c.libusb.getDeviceDesc(dev)
            if err != nil {
                c.libusb.dereference(dev)
                log.Print(err)
                continue
            }

            handle, err := c.libusb.open(dev)
            if err != nil {
                c.libusb.dereference(dev)
                log.Print(err)
                continue
            }
            o := &Device{handle: handle, ctx: c, Desc: desc}

            c.mu.Lock()
            c.devices[o] = true
            c.mu.Unlock()
            err = o.Close()
            if err != nil {
                println(err)
            }
            c.libusb.dereference(dev)

        }

        err = c.Close()
        if err != nil {
            log.Print(err)
        }
    }
}

[danielpaulus]

I think I found it. There seems to be no call to c.libusb.dereference(dev) in the leaking case. According to the libusb docs

libusb_open() adds another reference which is later destroyed by libusb_close().

So the code of OpenDevices should probably be changed to:

func (c *Context) OpenDevices(opener func(desc *DeviceDesc) bool) ([]*Device, error) {
    if c.ctx == nil {
        return nil, errors.New("OpenDevices called on a closed or uninitialized Context")
    }
    list, err := c.libusb.getDevices(c.ctx)
    if err != nil {
        return nil, err
    }

    var reterr error
    var ret []*Device
    for _, dev := range list {
        desc, err := c.libusb.getDeviceDesc(dev)
        if err != nil {
            c.libusb.dereference(dev)
            reterr = err
            continue
        }

        if opener(desc) {
            handle, err := c.libusb.open(dev)
            if err != nil {
                c.libusb.dereference(dev)
                reterr = err
                continue
            }
            o := &Device{handle: handle, ctx: c, Desc: desc}
            ret = append(ret, o)
            c.mu.Lock()
            c.devices[o] = true
            c.mu.Unlock()
        } 
//always deref the device to prevent memory leaks, the libusb.open call keeps a reference count already which will
//be reduced on device.close        
c.libusb.dereference(dev)

    }
    return ret, reterr
}

I will create a PR :-)