missing Close() method for VirtualNetwork

[urish]

Currently, there's no way to free the memory allocated by VirtualNetwork.

For instance, the following code will leak around ~100MB of memory:

    config := types.Configuration{
        Debug:             false,
        CaptureFile:       "",
        MTU:               1500,
        Subnet:            "10.10.10.0/16",
        GatewayIP:         "10.10.10.1",
        GatewayMacAddress: "12:34:56:78:aa:bb",
        DHCPStaticLeases:  map[string]string{},
        DNS:               []types.Zone{},
        Forwards:          map[string]string{},
        NAT:               map[string]string{},
        GatewayVirtualIPs: []string{"10.10.10.254"},
        Protocol:          types.QemuProtocol,
    }

    fmt.Println("Memory usage before creating virtual networks:")
    memStats()

    for i := 0; i < 1000; i++ {
        _, err := virtualnetwork.New(&config)
        if err != nil {
            panic(err)
        }
    }

    runtime.GC()

    fmt.Println("Memory usage after creating virtual networks:")
    memStats()

Output:

Memory usage before creating virtual networks:
Alloc = 0 MiB   TotalAlloc = 0 MiB      Sys = 10 MiB    NumGC = 0
Memory usage after creating virtual networks:
Alloc = 97 MiB  TotalAlloc = 125 MiB    Sys = 410 MiB   NumGC = 6

The implementation of memStats() (taken from here):

func memStats() {
    var m runtime.MemStats
    runtime.ReadMemStats(&m)
    // For info on each, see: https://golang.org/pkg/runtime/#MemStats
    fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
    fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
    fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
    fmt.Printf("\tNumGC = %v\n", m.NumGC)

}

func bToMb(b uint64) uint64 {
    return b / 1024 / 1024
}

[urish]

Created a small GitHub repo with a reproduction:

https://github.com/urish/virtualnetwork-leak

It's basically the same code as above, but packaged into a go project for easier reproduction.

[gbraad]

Thanks. Sorry for not noticing this earlier, but I will take a look at this.

[urish]

Thanks!

[gbraad]

The code as in the example

    for i := 0; i < 1000; i++ {
        _, err := virtualnetwork.New(&config)
        if err != nil {
            panic(err)
        }
    }

    runtime.GC()

just claims a new device and never release, hence an out-of-memory. The queston however is when you want to release it. Opening multiple is allowed and is hold on until released/closed. At what point would you make that decision?

[urish]

From my point of view, having a Close() method which would release a VirtualNetwork would be ideal

[gbraad]

OK, just to confirm that the issue is about adding additional functionality to deconstruct. :+1

On Fri, Dec 2, 2022 at 2:23 PM Uri Shaked @.***> wrote:

From my point of view, having a Close() method which would release a VirtualNetwork would be ideal

— Reply to this email directly, view it on GitHub https://github.com/containers/gvisor-tap-vsock/issues/161#issuecomment-1334814236, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAOZRUSWCPD3CPBC7JMZDWLGIUTANCNFSM6AAAAAASHPZ3JI . You are receiving this because you were assigned.Message ID: @.***>

--

Gerard Braad | http://gbraad.nl

STEM is the source, but BUILD is how you develop! [ Better Understanding Involves Learning and Doing ]

[urish]

Yes 😀

[cfergeau]

Spent some time on this, and most of the memory consumption is coming from VirtualNetwork.stack which has a Close() method. However, adding a VirtualNetwork.Close() with just a call to n.stack.Close() is not enough as this then results in dozens of ERRO[0000] EOF being logged. This has to do with what is done in pkg/virtualnetwork.addServices() as commenting out the call in pkg/virtualnetwork.New() removes the logs, but I stopped the investigation there for now, easy to get lost in that code :-/

[gbraad]

will also have to spend more time on this. I see memory leaks on Windows, but they are caused by something else that fails first