Merge pull request #18796 from mheon/lock_debugging

Add support for lock debugging

cmd/podman/system/locks.go

@@ -0,0 +1,54 @@
+package system
+
+import (
+	"fmt"
+
+	"github.com/containers/podman/v4/cmd/podman/registry"
+	"github.com/containers/podman/v4/cmd/podman/validate"
+	"github.com/spf13/cobra"
+)
+
+var (
+	locksCommand = &cobra.Command{
+		Use:    "locks",
+		Short:  "Debug Libpod's use of locks, identifying any potential conflicts",
+		Args:   validate.NoArgs,
+		Hidden: true,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return runLocks()
+		},
+		Example: "podman system locks",
+	}
+)
+
+func init() {
+	registry.Commands = append(registry.Commands, registry.CliCommand{
+		Command: locksCommand,
+		Parent:  systemCmd,
+	})
+}
+func runLocks() error {
+	report, err := registry.ContainerEngine().Locks(registry.Context())
+	if err != nil {
+		return err
+	}
+
+	for lockNum, objects := range report.LockConflicts {
+		fmt.Printf("Lock %d is in use by the following\n:", lockNum)
+		for _, obj := range objects {
+			fmt.Printf("\t%s\n", obj)
+		}
+	}
+
+	if len(report.LockConflicts) > 0 {
+		fmt.Printf("\nLock conflicts have been detected. Recommend immediate use of `podman system renumber` to resolve.\n\n")
+	} else {
+		fmt.Printf("\nNo lock conflicts have been detected.\n\n")
+	}
+
+	for _, lockNum := range report.LocksHeld {
+		fmt.Printf("Lock %d is presently being held\n", lockNum)
+	}
+
+	return nil
+}

docs/source/markdown/podman-container-inspect.1.md.in

@@ -43,6 +43,7 @@ Valid placeholders for the Go template are listed below:
 | .IsInfra                 | Is this an infra container? (string: true/false)   |
 | .IsService               | Is this a service container? (string: true/false)  |
 | .KubeExitCodePropagation | Kube exit-code propagation (string)                |
+| .LockNumber              | Number of the container's Libpod lock              |
 | .MountLabel              | SELinux label of mount (string)                    |
 | .Mounts                  | Mounts (array of strings)                          |
 | .Name                    | Container name (string)                            |

docs/source/markdown/podman-pod-inspect.1.md.in

@@ -44,6 +44,7 @@ Valid placeholders for the Go template are listed below:
 | .InfraContainerID    | Pod infrastructure ID                       |
 | .InspectPodData ...  | Nested structure, for experts only          |
 | .Labels              | Pod labels                                  |
+| .LockNumber          | Number of the pod's Libpod lock             |
 | .MemoryLimit         | Memory limit, bytes                         |
 | .MemorySwap          | Memory swap limit, in bytes                 |
 | .Mounts              | Mounts                                      |

docs/source/markdown/podman-volume-inspect.1.md

@@ -33,6 +33,7 @@ Valid placeholders for the Go template are listed below:
 | .Driver             | Volume driver                                          |
 | .GID                | GID the volume was created with                        |
 | .Labels             | Label information associated with the volume           |
+| .LockNumber         | Number of the volume's Libpod lock                     |
 | .MountCount         | Number of times the volume is mounted                  |
 | .Mountpoint         | Source of volume mount point                           |
 | .Name               | Volume name                                            |

docs/source/markdown/podman-volume-ls.1.md.in

@@ -42,6 +42,7 @@ Valid placeholders for the Go template are listed below:
 | .GID                      | GID of volume                                |
 | .InspectVolumeData ...    | Don't use                                    |
 | .Labels                   | Label information associated with the volume |
+| .LockNumber               | Number of the volume's Libpod lock           |
 | .MountCount               | Number of times the volume is mounted        |
 | .Mountpoint               | Source of volume mount point                 |
 | .Name                     | Volume name                                  |

libpod/container_inspect.go

@@ -166,6 +166,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver
 		IsInfra:                 c.IsInfra(),
 		IsService:               c.IsService(),
 		KubeExitCodePropagation: config.KubeExitCodePropagation.String(),
+		LockNumber:              c.lock.ID(),
 	}
 
 	if config.RootfsImageID != "" { // May not be set if the container was created with --rootfs

libpod/define/container_inspect.go

@@ -691,6 +691,7 @@ type InspectContainerData struct {
 	IsInfra                 bool                        `json:"IsInfra"`
 	IsService               bool                        `json:"IsService"`
 	KubeExitCodePropagation string                      `json:"KubeExitCodePropagation"`
+	LockNumber              uint32                      `json:"lockNumber"`
 	Config                  *InspectContainerConfig     `json:"Config"`
 	HostConfig              *InspectContainerHostConfig `json:"HostConfig"`
 }

libpod/define/info.go

@@ -38,6 +38,7 @@ type HostInfo struct {
 	DatabaseBackend   string           `json:"databaseBackend"`
 	Distribution      DistributionInfo `json:"distribution"`
 	EventLogger       string           `json:"eventLogger"`
+	FreeLocks         *uint32          `json:"freeLocks,omitempty"`
 	Hostname          string           `json:"hostname"`
 	IDMappings        IDMappings       `json:"idMappings,omitempty"`
 	Kernel            string           `json:"kernel"`

libpod/define/pod_inspect.go

@@ -85,6 +85,8 @@ type InspectPodData struct {
 	BlkioWeightDevice []InspectBlkioWeightDevice `json:"blkio_weight_device,omitempty"`
 	// RestartPolicy of the pod.
 	RestartPolicy string `json:"RestartPolicy,omitempty"`
+	// Number of the pod's Libpod lock.
+	LockNumber uint32
 }
 
 // InspectPodInfraConfig contains the configuration of the pod's infra

libpod/define/volume_inspect.go

@@ -61,6 +61,8 @@ type InspectVolumeData struct {
 	// StorageID is the ID of the container backing the volume in c/storage.
 	// Only used with Image Volumes.
 	StorageID string `json:"StorageID,omitempty"`
+	// LockNumber is the number of the volume's Libpod lock.
+	LockNumber uint32
 }
 
 type VolumeReload struct {

libpod/info.go

@@ -99,6 +99,12 @@ func (r *Runtime) hostInfo() (*define.HostInfo, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	locksFree, err := r.lockManager.AvailableLocks()
+	if err != nil {
+		return nil, fmt.Errorf("getting free locks: %w", err)
+	}
+
 	info := define.HostInfo{
 		Arch:            runtime.GOARCH,
 		BuildahVersion:  buildah.Version,
@@ -107,6 +113,7 @@ func (r *Runtime) hostInfo() (*define.HostInfo, error) {
 		CPUs:            runtime.NumCPU(),
 		CPUUtilization:  cpuUtil,
 		Distribution:    hostDistributionInfo,
+		FreeLocks:       locksFree,
 		LogDriver:       r.config.Containers.LogDriver,
 		EventLogger:     r.eventer.String(),
 		Hostname:        host,

libpod/lock/file_lock_manager.go

@@ -1,6 +1,7 @@
 package lock
 
 import (
+	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/libpod/lock/file"
 )
 
@@ -79,6 +80,21 @@ func (m *FileLockManager) FreeAllLocks() error {
 	return m.locks.DeallocateAllLocks()
 }
 
+// AvailableLocks returns the number of available locks. Since this is not
+// limited in the file lock implementation, nil is returned.
+func (m *FileLockManager) AvailableLocks() (*uint32, error) {
+	return nil, nil
+}
+
+// LocksHeld returns any locks that are presently locked.
+// It is not implemented for the file lock backend.
+// It ought to be possible, but my motivation to dig into c/storage and add
+// trylock semantics to the filelocker implementation for an uncommonly-used
+// lock backend is lacking.
+func (m *FileLockManager) LocksHeld() ([]uint32, error) {
+	return nil, define.ErrNotImplemented
+}
+
 // FileLock is an individual shared memory lock.
 type FileLock struct {
 	lockID  uint32

libpod/lock/in_memory_locks.go

@@ -116,3 +116,33 @@ func (m *InMemoryManager) FreeAllLocks() error {
 
 	return nil
 }
+
+// Get number of available locks
+func (m *InMemoryManager) AvailableLocks() (*uint32, error) {
+	var count uint32
+
+	for _, lock := range m.locks {
+		if !lock.allocated {
+			count++
+		}
+	}
+
+	return &count, nil
+}
+
+// Get any locks that are presently being held.
+// Useful for debugging deadlocks.
+func (m *InMemoryManager) LocksHeld() ([]uint32, error) {
+	//nolint:prealloc
+	var locks []uint32
+
+	for _, lock := range m.locks {
+		if lock.lock.TryLock() {
+			lock.lock.Unlock()
+			continue
+		}
+		locks = append(locks, lock.ID())
+	}
+
+	return locks, nil
+}

libpod/lock/lock.go

@@ -45,6 +45,16 @@ type Manager interface {
 	// renumbering, where reasonable guarantees about other processes can be
 	// made.
 	FreeAllLocks() error
+	// NumAvailableLocks gets the number of remaining locks available to be
+	// allocated.
+	// Some lock managers do not have a maximum number of locks, and can
+	// allocate an unlimited number. These implementations should return
+	// a nil uin32.
+	AvailableLocks() (*uint32, error)
+	// Get a list of locks that are currently locked.
+	// This may not be supported by some drivers, depending on the exact
+	// backend implementation in use.
+	LocksHeld() ([]uint32, error)
 }
 
 // Locker is similar to sync.Locker, but provides a method for freeing the lock

libpod/lock/shm/shm_lock.c

@@ -20,12 +20,18 @@ static size_t compute_shm_size(uint32_t num_bitmaps) {
 // Handles exceptional conditions, including a mutex locked by a process that
 // died holding it.
 // Returns 0 on success, or positive errno on failure.
-static int take_mutex(pthread_mutex_t *mutex) {
+static int take_mutex(pthread_mutex_t *mutex, bool trylock) {
   int ret_code;
 
-  do {
-    ret_code = pthread_mutex_lock(mutex);
-  } while(ret_code == EAGAIN);
+  if (!trylock) {
+    do {
+      ret_code = pthread_mutex_lock(mutex);
+    } while(ret_code == EAGAIN);
+  } else {
+    do {
+      ret_code = pthread_mutex_trylock(mutex);
+    } while(ret_code == EAGAIN);
+  }
 
   if (ret_code == EOWNERDEAD) {
     // The previous owner of the mutex died while holding it
@@ -309,7 +315,7 @@ int64_t allocate_semaphore(shm_struct_t *shm) {
   }
 
   // Lock the semaphore controlling access to our shared memory
-  ret_code = take_mutex(&(shm->segment_lock));
+  ret_code = take_mutex(&(shm->segment_lock), false);
   if (ret_code != 0) {
     return -1 * ret_code;
   }
@@ -383,7 +389,7 @@ int32_t allocate_given_semaphore(shm_struct_t *shm, uint32_t sem_index) {
   test_map = 0x1 << index_in_bitmap;
 
   // Lock the mutex controlling access to our shared memory
-  ret_code = take_mutex(&(shm->segment_lock));
+  ret_code = take_mutex(&(shm->segment_lock), false);
   if (ret_code != 0) {
     return -1 * ret_code;
   }
@@ -436,7 +442,7 @@ int32_t deallocate_semaphore(shm_struct_t *shm, uint32_t sem_index) {
   test_map = 0x1 << index_in_bitmap;
 
   // Lock the mutex controlling access to our shared memory
-  ret_code = take_mutex(&(shm->segment_lock));
+  ret_code = take_mutex(&(shm->segment_lock), false);
   if (ret_code != 0) {
     return -1 * ret_code;
   }
@@ -475,7 +481,7 @@ int32_t deallocate_all_semaphores(shm_struct_t *shm) {
   }
 
   // Lock the mutex controlling access to our shared memory
-  ret_code = take_mutex(&(shm->segment_lock));
+  ret_code = take_mutex(&(shm->segment_lock), false);
   if (ret_code != 0) {
     return -1 * ret_code;
   }
@@ -513,7 +519,7 @@ int32_t lock_semaphore(shm_struct_t *shm, uint32_t sem_index) {
   bitmap_index = sem_index / BITMAP_SIZE;
   index_in_bitmap = sem_index % BITMAP_SIZE;
 
-  return -1 * take_mutex(&(shm->locks[bitmap_index].locks[index_in_bitmap]));
+  return -1 * take_mutex(&(shm->locks[bitmap_index].locks[index_in_bitmap]), false);
 }
 
 // Unlock a given semaphore
@@ -537,3 +543,98 @@ int32_t unlock_semaphore(shm_struct_t *shm, uint32_t sem_index) {
 
   return -1 * release_mutex(&(shm->locks[bitmap_index].locks[index_in_bitmap]));
 }
+
+// Get the number of free locks.
+// Returns a positive integer guaranteed to be less than UINT32_MAX on success,
+// or negative errno values on failure.
+// On success, the returned integer is the number of free semaphores.
+int64_t available_locks(shm_struct_t *shm) {
+  int ret_code, i, count;
+  bitmap_t test_map;
+  int64_t free_locks = 0;
+
+  if (shm == NULL) {
+    return -1 * EINVAL;
+  }
+
+  // Lock the semaphore controlling access to the SHM segment.
+  // This isn't strictly necessary as we're only reading, but it seems safer.
+  ret_code = take_mutex(&(shm->segment_lock), false);
+  if (ret_code != 0) {
+    return -1 * ret_code;
+  }
+
+  // Loop through all bitmaps, counting number of allocated locks.
+  for (i = 0; i < shm->num_bitmaps; i++) {
+    // Short-circuit to catch fully-empty bitmaps quick.
+    if (shm->locks[i].bitmap == 0) {
+      free_locks += sizeof(bitmap_t) * 8;
+      continue;
+    }
+
+    // Use Kernighan's Algorithm to count bits set. Subtract from number of bits
+    // in the integer to get free bits, and thus free lock count.
+    test_map = shm->locks[i].bitmap;
+    count = 0;
+    while (test_map) {
+      test_map = test_map & (test_map - 1);
+      count++;
+    }
+
+    free_locks += (sizeof(bitmap_t) * 8) - count;
+  }
+
+  // Clear the mutex
+  ret_code = release_mutex(&(shm->segment_lock));
+  if (ret_code != 0) {
+    return -1 * ret_code;
+  }
+
+  // Return free lock count.
+  return free_locks;
+}
+
+// Attempt to take a given semaphore. If successfully taken, it is immediately
+// released before the function returns.
+// Used to check if a semaphore is in use, to detect potential deadlocks where a
+// lock has not been released for an extended period of time.
+// Note that this is NOT POSIX trylock as the lock is immediately released if
+// taken.
+// Returns negative errno on failure.
+// On success, returns 1 if the lock was successfully taken, and 0 if it was
+// not.
+int32_t try_lock(shm_struct_t *shm, uint32_t sem_index) {
+  int bitmap_index, index_in_bitmap, ret_code;
+  pthread_mutex_t *mutex;
+
+  if (shm == NULL) {
+    return -1 * EINVAL;
+  }
+
+  if (sem_index >= shm->num_locks) {
+    return -1 * EINVAL;
+  }
+
+  bitmap_index = sem_index / BITMAP_SIZE;
+  index_in_bitmap = sem_index % BITMAP_SIZE;
+
+  mutex = &(shm->locks[bitmap_index].locks[index_in_bitmap]);
+
+  ret_code = take_mutex(mutex, true);
+
+  if (ret_code == EBUSY) {
+    // Did not successfully take the lock
+    return 0;
+  } else if (ret_code != 0) {
+    // Another, unrelated error
+    return -1 * ret_code;
+  }
+
+  // Lock taken successfully, unlock and return.
+  ret_code = release_mutex(mutex);
+  if (ret_code != 0) {
+    return -1 * ret_code;
+  }
+
+  return 1;
+}