[lxc-devel] [lxd/master] GC improvements

[stgraber on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180601/f6245061/attachment.bin>
-------------- next part --------------
From d70c1385eba7aeeabf7546bccc22ad6076dcf6bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 31 May 2018 23:51:32 -0400
Subject: [PATCH 1/2] Manually release the liblxc structs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/container_lxc.go | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 6c008abae..aadde3e56 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -13,6 +13,7 @@ import (
 	"path"
 	"path/filepath"
 	"reflect"
+	"runtime"
 	"sort"
 	"strconv"
 	"strings"
@@ -466,6 +467,9 @@ func containerLXCLoad(s *state.State, args db.ContainerArgs) (container, error)
 	// Create the container struct
 	c := containerLXCInstantiate(s, args)
 
+	// Setup finalizer
+	runtime.SetFinalizer(c, containerLXCUnload)
+
 	// Load the config.
 	err := c.init()
 	if err != nil {
@@ -475,6 +479,15 @@ func containerLXCLoad(s *state.State, args db.ContainerArgs) (container, error)
 	return c, nil
 }
 
+// Unload is called by the garbage collector
+func containerLXCUnload(c *containerLXC) {
+	runtime.SetFinalizer(c, nil)
+	if c.c != nil {
+		lxc.Release(c.c)
+		c.c = nil
+	}
+}
+
 // Create a container struct without initializing it.
 func containerLXCInstantiate(s *state.State, args db.ContainerArgs) *containerLXC {
 	return &containerLXC{
@@ -7166,6 +7179,8 @@ func (c *containerLXC) fillNetworkDevice(name string, m types.Device) (types.Dev
 		// Attempt to include all existing interfaces
 		cc, err := lxc.NewContainer(c.Name(), c.state.OS.LxcPath)
 		if err == nil {
+			defer lxc.Release(cc)
+
 			interfaces, err := cc.Interfaces()
 			if err == nil {
 				for _, name := range interfaces {
@@ -7423,6 +7438,7 @@ func (c *containerLXC) removeNetworkDevice(name string, m types.Device) error {
 	if err != nil {
 		return err
 	}
+	defer lxc.Release(cc)
 
 	// Remove the interface from the container
 	err = cc.DetachInterfaceRename(m["name"], hostName)

From 88073c1168675d58bb5cb3c48e8b8572a256c477 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 31 May 2018 16:53:05 -0400
Subject: [PATCH 2/2] Drop manual GC calls
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The GC has come a long way since we had to add those calls to avoid fd
leaks when getting rid of references to the lxc struct.

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/daemon.go     | 13 -------------
 lxd/operations.go | 13 -------------
 2 files changed, 26 deletions(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index a0342dcb4..d1a30fcfb 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -11,7 +11,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 	"sync"
 	"syscall"
@@ -318,18 +317,6 @@ func (d *Daemon) createCmd(restAPI *mux.Router, version string, c Command) {
 				logger.Errorf("Failed writing error for error, giving up")
 			}
 		}
-
-		/*
-		 * When we create a new lxc.Container, it adds a finalizer (via
-		 * SetFinalizer) that frees the struct. However, it sometimes
-		 * takes the go GC a while to actually free the struct,
-		 * presumably since it is a small amount of memory.
-		 * Unfortunately, the struct also keeps the log fd open, so if
-		 * we leave too many of these around, we end up running out of
-		 * fds. So, let's explicitly do a GC to collect these at the
-		 * end of each request.
-		 */
-		runtime.GC()
 	})
 }
 
diff --git a/lxd/operations.go b/lxd/operations.go
index 466e28896..849256ed2 100644
--- a/lxd/operations.go
+++ b/lxd/operations.go
@@ -3,7 +3,6 @@ package main
 import (
 	"fmt"
 	"net/http"
-	"runtime"
 	"strings"
 	"sync"
 	"time"
@@ -100,18 +99,6 @@ func (op *operation) done() {
 		if err != nil {
 			logger.Warnf("Failed to delete operation %s: %s", op.id, err)
 		}
-
-		/*
-		 * When we create a new lxc.Container, it adds a finalizer (via
-		 * SetFinalizer) that frees the struct. However, it sometimes
-		 * takes the go GC a while to actually free the struct,
-		 * presumably since it is a small amount of memory.
-		 * Unfortunately, the struct also keeps the log fd open, so if
-		 * we leave too many of these around, we end up running out of
-		 * fds. So, let's explicitly do a GC to collect these at the
-		 * end of each request.
-		 */
-		runtime.GC()
 	})
 }