diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go index 8d2aa5da439..91c314e09ea 100644 --- a/libcontainer/cgroups/systemd/common.go +++ b/libcontainer/cgroups/systemd/common.go @@ -384,15 +384,23 @@ func setUnitProperties(cm *dbusConnManager, name string, properties ...systemdDb }) } +func getManagerProperty(cm *dbusConnManager, name string) (string, error) { + str := "" + err := cm.retryOnDisconnect(func(c *systemdDbus.Conn) error { + var err error + str, err = c.GetManagerProperty(name) + return err + }) + if err != nil { + return "", err + } + return strconv.Unquote(str) +} + func systemdVersion(cm *dbusConnManager) int { versionOnce.Do(func() { version = -1 - var verStr string - err := cm.retryOnDisconnect(func(c *systemdDbus.Conn) error { - var err error - verStr, err = c.GetManagerProperty("Version") - return err - }) + verStr, err := getManagerProperty(cm, "Version") if err == nil { version, err = systemdVersionAtoi(verStr) } @@ -407,11 +415,11 @@ func systemdVersion(cm *dbusConnManager) int { func systemdVersionAtoi(verStr string) (int, error) { // verStr should be of the form: - // "v245.4-1.fc32", "245", "v245-1.fc32", "245-1.fc32" - // all the input strings include quotes, and the output int should be 245 - // thus, we unconditionally remove the `"v` - // and then match on the first integer we can grab - re := regexp.MustCompile(`"?v?([0-9]+)`) + // "v245.4-1.fc32", "245", "v245-1.fc32", "245-1.fc32" (without quotes). + // The result for all of the above should be 245. + // Thus, we unconditionally remove the "v" prefix + // and then match on the first integer we can grab. + re := regexp.MustCompile(`v?([0-9]+)`) matches := re.FindStringSubmatch(verStr) if len(matches) < 2 { return 0, errors.Errorf("can't parse version %s: incorrect number of matches %v", verStr, matches) diff --git a/libcontainer/cgroups/systemd/dbus.go b/libcontainer/cgroups/systemd/dbus.go index deca16b005b..0f7406cd9d6 100644 --- a/libcontainer/cgroups/systemd/dbus.go +++ b/libcontainer/cgroups/systemd/dbus.go @@ -10,37 +10,43 @@ import ( dbus "github.com/godbus/dbus/v5" ) +var ( + dbusC *systemdDbus.Conn + dbusMu sync.RWMutex + dbusInited bool + dbusRootless bool +) + type dbusConnManager struct { - conn *systemdDbus.Conn - rootless bool - sync.RWMutex } // newDbusConnManager initializes systemd dbus connection manager. func newDbusConnManager(rootless bool) *dbusConnManager { - return &dbusConnManager{ - rootless: rootless, + if dbusInited && rootless != dbusRootless { + panic("can't have both root and rootless dbus") } + dbusRootless = rootless + return &dbusConnManager{} } // getConnection lazily initializes and returns systemd dbus connection. func (d *dbusConnManager) getConnection() (*systemdDbus.Conn, error) { - // In the case where d.conn != nil + // In the case where dbusC != nil // Use the read lock the first time to ensure // that Conn can be acquired at the same time. - d.RLock() - if conn := d.conn; conn != nil { - d.RUnlock() + dbusMu.RLock() + if conn := dbusC; conn != nil { + dbusMu.RUnlock() return conn, nil } - d.RUnlock() + dbusMu.RUnlock() - // In the case where d.conn == nil + // In the case where dbusC == nil // Use write lock to ensure that only one // will be created - d.Lock() - defer d.Unlock() - if conn := d.conn; conn != nil { + dbusMu.Lock() + defer dbusMu.Unlock() + if conn := dbusC; conn != nil { return conn, nil } @@ -48,12 +54,12 @@ func (d *dbusConnManager) getConnection() (*systemdDbus.Conn, error) { if err != nil { return nil, err } - d.conn = conn + dbusC = conn return conn, nil } func (d *dbusConnManager) newConnection() (*systemdDbus.Conn, error) { - if d.rootless { + if dbusRootless { return newUserSystemdDbus() } return systemdDbus.NewWithContext(context.TODO()) @@ -62,11 +68,11 @@ func (d *dbusConnManager) newConnection() (*systemdDbus.Conn, error) { // resetConnection resets the connection to its initial state // (so it can be reconnected if necessary). func (d *dbusConnManager) resetConnection(conn *systemdDbus.Conn) { - d.Lock() - defer d.Unlock() - if d.conn != nil && d.conn == conn { - d.conn.Close() - d.conn = nil + dbusMu.Lock() + defer dbusMu.Unlock() + if dbusC != nil && dbusC == conn { + dbusC.Close() + dbusC = nil } } diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index 9dcde4233b1..99f3e450e93 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/libcontainer/cgroups/systemd/v2.go @@ -337,16 +337,8 @@ func (m *unifiedManager) getSliceFull() (string, error) { } if m.rootless { - dbusConnection, err := m.dbus.getConnection() - if err != nil { - return "", err - } - // managerCGQuoted is typically "/user.slice/user-${uid}.slice/user@${uid}.service" including the quote symbols - managerCGQuoted, err := dbusConnection.GetManagerProperty("ControlGroup") - if err != nil { - return "", err - } - managerCG, err := strconv.Unquote(managerCGQuoted) + // managerCG is typically "/user.slice/user-${uid}.slice/user@${uid}.service". + managerCG, err := getManagerProperty(m.dbus, "ControlGroup") if err != nil { return "", err } diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 499a5b813dd..505006a89ac 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -1917,6 +1917,14 @@ func TestCGROUPHost(t *testing.T) { } func TestFdLeaks(t *testing.T) { + testFdLeaks(t, false) +} + +func TestFdLeaksSystemd(t *testing.T) { + testFdLeaks(t, true) +} + +func testFdLeaks(t *testing.T, systemd bool) { if testing.Short() { return } @@ -1933,7 +1941,10 @@ func TestFdLeaks(t *testing.T) { _, err = pfd.Seek(0, 0) ok(t, err) - config := newTemplateConfig(t, &tParam{rootfs: rootfs}) + config := newTemplateConfig(t, &tParam{ + rootfs: rootfs, + systemd: systemd, + }) buffers, exitCode, err := runContainer(t, config, "", "true") ok(t, err)