Merge pull request #2203 from mrunalp/systemd_conn_cleanup

Separate systemd dbus connection initialization from running check

libcontainer/cgroups/systemd/apply_nosystemd.go

@@ -14,7 +14,7 @@ type Manager struct {
 	Paths   map[string]string
 }
 
-func UseSystemd() bool {
+func IsRunningSystemd() bool {
 	return false
 }
 

libcontainer/cgroups/systemd/apply_systemd.go

@@ -71,8 +71,9 @@ const (
 )
 
 var (
-	connLock sync.Mutex
-	theConn  *systemdDbus.Conn
+	connOnce sync.Once
+	connDbus *systemdDbus.Conn
+	connErr  error
 )
 
 func newProp(name string, units interface{}) systemdDbus.Property {
@@ -89,34 +90,25 @@ func newProp(name string, units interface{}) systemdDbus.Property {
 // system. This functions similarly to systemd's `sd_booted(3)`: internally, it
 // checks whether /run/systemd/system/ exists and is a directory.
 // http://www.freedesktop.org/software/systemd/man/sd_booted.html
-func isRunningSystemd() bool {
+func IsRunningSystemd() bool {
 	fi, err := os.Lstat("/run/systemd/system")
 	if err != nil {
 		return false
 	}
 	return fi.IsDir()
 }
 
-func UseSystemd() bool {
-	if !isRunningSystemd() {
-		return false
-	}
-
-	connLock.Lock()
-	defer connLock.Unlock()
-
-	if theConn == nil {
-		var err error
-		theConn, err = systemdDbus.New()
-		if err != nil {
-			return false
-		}
-	}
-	return true
+// getDbusConnection lazy initializes systemd dbus connection
+// and returns it
+func getDbusConnection() (*systemdDbus.Conn, error) {
+	connOnce.Do(func() {
+		connDbus, connErr = systemdDbus.New()
+	})
+	return connDbus, connErr
 }
 
 func NewSystemdCgroupsManager() (func(config *configs.Cgroup, paths map[string]string) cgroups.Manager, error) {
-	if !isRunningSystemd() {
+	if !IsRunningSystemd() {
 		return nil, fmt.Errorf("systemd not running on this host, can't use systemd as a cgroups.Manager")
 	}
 	if cgroups.IsCgroup2UnifiedMode() {
@@ -247,8 +239,12 @@ func (m *LegacyManager) Apply(pid int) error {
 
 	properties = append(properties, c.SystemdProps...)
 
+	dbusConnection, err := getDbusConnection()
+	if err != nil {
+		return err
+	}
 	statusChan := make(chan string, 1)
-	if _, err := theConn.StartTransientUnit(unitName, "replace", properties, statusChan); err == nil {
+	if _, err := dbusConnection.StartTransientUnit(unitName, "replace", properties, statusChan); err == nil {
 		select {
 		case <-statusChan:
 		case <-time.After(time.Second):
@@ -284,7 +280,13 @@ func (m *LegacyManager) Destroy() error {
 	}
 	m.mu.Lock()
 	defer m.mu.Unlock()
-	theConn.StopUnit(getUnitName(m.Cgroups), "replace", nil)
+
+	dbusConnection, err := getDbusConnection()
+	if err != nil {
+		return err
+	}
+
+	dbusConnection.StopUnit(getUnitName(m.Cgroups), "replace", nil)
 	if err := cgroups.RemovePaths(m.Paths); err != nil {
 		return err
 	}

libcontainer/cgroups/systemd/unified_hierarchy.go

@@ -119,8 +119,13 @@ func (m *UnifiedManager) Apply(pid int) error {
 
 	// ignore c.Resources.KernelMemory
 
+	dbusConnection, err := getDbusConnection()
+	if err != nil {
+		return err
+	}
+
 	statusChan := make(chan string, 1)
-	if _, err := theConn.StartTransientUnit(unitName, "replace", properties, statusChan); err == nil {
+	if _, err := dbusConnection.StartTransientUnit(unitName, "replace", properties, statusChan); err == nil {
 		select {
 		case <-statusChan:
 		case <-time.After(time.Second):
@@ -155,7 +160,12 @@ func (m *UnifiedManager) Destroy() error {
 	}
 	m.mu.Lock()
 	defer m.mu.Unlock()
-	theConn.StopUnit(getUnitName(m.Cgroups), "replace", nil)
+
+	dbusConnection, err := getDbusConnection()
+	if err != nil {
+		return err
+	}
+	dbusConnection.StopUnit(getUnitName(m.Cgroups), "replace", nil)
 	if err := cgroups.RemovePaths(m.Paths); err != nil {
 		return err
 	}

libcontainer/integration/exec_test.go

@@ -506,7 +506,7 @@ func TestFreeze(t *testing.T) {
 }
 
 func TestSystemdFreeze(t *testing.T) {
-	if !systemd.UseSystemd() {
+	if !systemd.IsRunningSystemd() {
 		t.Skip("Systemd is unsupported")
 	}
 	testFreeze(t, true)
@@ -563,7 +563,7 @@ func TestCpuShares(t *testing.T) {
 }
 
 func TestCpuSharesSystemd(t *testing.T) {
-	if !systemd.UseSystemd() {
+	if !systemd.IsRunningSystemd() {
 		t.Skip("Systemd is unsupported")
 	}
 	testCpuShares(t, true)
@@ -598,7 +598,7 @@ func TestPids(t *testing.T) {
 }
 
 func TestPidsSystemd(t *testing.T) {
-	if !systemd.UseSystemd() {
+	if !systemd.IsRunningSystemd() {
 		t.Skip("Systemd is unsupported")
 	}
 	testPids(t, true)
@@ -684,7 +684,7 @@ func TestRunWithKernelMemory(t *testing.T) {
 }
 
 func TestRunWithKernelMemorySystemd(t *testing.T) {
-	if !systemd.UseSystemd() {
+	if !systemd.IsRunningSystemd() {
 		t.Skip("Systemd is unsupported")
 	}
 	testRunWithKernelMemory(t, true)

utils_linux.go

@@ -47,7 +47,7 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) {
 		cgroupManager = libcontainer.RootlessCgroupfs
 	}
 	if context.GlobalBool("systemd-cgroup") {
-		if systemd.UseSystemd() {
+		if systemd.IsRunningSystemd() {
 			cgroupManager = libcontainer.SystemdCgroups
 		} else {
 			return nil, fmt.Errorf("systemd cgroup flag passed, but systemd support for managing cgroups is not available")