diff options
Diffstat (limited to 'libpod/lock')
| -rw-r--r-- | libpod/lock/lock.go | 8 | ||||
| -rw-r--r-- | libpod/lock/shm/shm_lock.c | 245 | ||||
| -rw-r--r-- | libpod/lock/shm/shm_lock.go | 52 | ||||
| -rw-r--r-- | libpod/lock/shm/shm_lock.h | 35 | ||||
| -rw-r--r-- | libpod/lock/shm/shm_lock_test.go | 56 | ||||
| -rw-r--r-- | libpod/lock/shm_lock_manager_linux.go | 42 |
6 files changed, 269 insertions, 169 deletions
diff --git a/libpod/lock/lock.go b/libpod/lock/lock.go index 6d17828f6..5258c641f 100644 --- a/libpod/lock/lock.go +++ b/libpod/lock/lock.go @@ -23,7 +23,7 @@ type Manager interface { // RetrieveLock retrieves a lock given its UUID. // The underlying lock MUST be the same as another other lock with the // same UUID. - RetrieveLock(id string) (Locker, error) + RetrieveLock(id uint32) (Locker, error) } // Locker is similar to sync.Locker, but provides a method for freeing the lock @@ -37,7 +37,7 @@ type Locker interface { // ID is guaranteed to uniquely identify the lock within the // Manager - that is, calling RetrieveLock with this ID will return // another instance of the same lock. - ID() string + ID() uint32 // Lock locks the lock. // This call MUST block until it successfully acquires the lock or // encounters a fatal error. @@ -46,8 +46,8 @@ type Locker interface { // A call to Unlock() on a lock that is already unlocked lock MUST // error. Unlock() error - // Deallocate deallocates the underlying lock, allowing its reuse by - // other pods and containers. + // Free deallocates the underlying lock, allowing its reuse by other + // pods and containers. // The lock MUST still be usable after a Free() - some libpod instances // may still retain Container structs with the old lock. This simply // advises the manager that the lock may be reallocated. diff --git a/libpod/lock/shm/shm_lock.c b/libpod/lock/shm/shm_lock.c index 3fe41f63c..4af58d857 100644 --- a/libpod/lock/shm/shm_lock.c +++ b/libpod/lock/shm/shm_lock.c @@ -1,6 +1,6 @@ #include <errno.h> #include <fcntl.h> -#include <semaphore.h> +#include <pthread.h> #include <stdbool.h> #include <stdint.h> #include <stdlib.h> @@ -12,19 +12,67 @@ #include "shm_lock.h" // Compute the size of the SHM struct -size_t compute_shm_size(uint32_t num_bitmaps) { +static size_t compute_shm_size(uint32_t num_bitmaps) { return sizeof(shm_struct_t) + (num_bitmaps * sizeof(lock_group_t)); } +// Take the given mutex. +// 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) { + int ret_code; + + do { + ret_code = pthread_mutex_lock(mutex); + } while(ret_code == EAGAIN); + + if (ret_code == EOWNERDEAD) { + // The previous owner of the mutex died while holding it + // Take it for ourselves + ret_code = pthread_mutex_consistent(mutex); + if (ret_code != 0) { + // Someone else may have gotten here first and marked the state consistent + // However, the mutex could also be invalid. + // Fail here instead of looping back to trying to lock the mutex. + return ret_code; + } + } else if (ret_code != 0) { + return ret_code; + } + + return 0; +} + +// Release the given mutex. +// Returns 0 on success, or positive errno on failure. +static int release_mutex(pthread_mutex_t *mutex) { + int ret_code; + + do { + ret_code = pthread_mutex_unlock(mutex); + } while(ret_code == EAGAIN); + + if (ret_code != 0) { + return ret_code; + } + + return 0; +} + // Set up an SHM segment holding locks for libpod. -// num_locks must be a multiple of BITMAP_SIZE (32 by default). +// num_locks must not be 0. +// Path is the path to the SHM segment. It must begin with a single / and +// container no other / characters, and be at most 255 characters including +// terminating NULL byte. // Returns a valid pointer on success or NULL on error. -// If an error occurs, it will be written to the int pointed to by error_code. -shm_struct_t *setup_lock_shm(uint32_t num_locks, int *error_code) { +// If an error occurs, negative ERRNO values will be written to error_code. +shm_struct_t *setup_lock_shm(char *path, uint32_t num_locks, int *error_code) { int shm_fd, i, j, ret_code; uint32_t num_bitmaps; size_t shm_size; shm_struct_t *shm; + pthread_mutexattr_t attr; // If error_code doesn't point to anything, we can't reasonably return errors // So fail immediately @@ -34,67 +82,93 @@ shm_struct_t *setup_lock_shm(uint32_t num_locks, int *error_code) { // We need a nonzero number of locks if (num_locks == 0) { - *error_code = EINVAL; + *error_code = -1 * EINVAL; return NULL; } - // Calculate the number of bitmaps required - if (num_locks % BITMAP_SIZE != 0) { - // Number of locks not a multiple of BITMAP_SIZE - *error_code = EINVAL; + if (path == NULL) { + *error_code = -1 * EINVAL; return NULL; } + + // Calculate the number of bitmaps required num_bitmaps = num_locks / BITMAP_SIZE; + if (num_locks % BITMAP_SIZE != 0) { + // The actual number given is not an even multiple of our bitmap size + // So round up + num_bitmaps += 1; + } // Calculate size of the shm segment shm_size = compute_shm_size(num_bitmaps); // Create a new SHM segment for us - shm_fd = shm_open(SHM_NAME, O_RDWR | O_CREAT | O_EXCL, 0600); + shm_fd = shm_open(path, O_RDWR | O_CREAT | O_EXCL, 0600); if (shm_fd < 0) { - *error_code = errno; + *error_code = -1 * errno; return NULL; } // Increase its size to what we need ret_code = ftruncate(shm_fd, shm_size); if (ret_code < 0) { - *error_code = errno; + *error_code = -1 * errno; goto CLEANUP_UNLINK; } // Map the shared memory in shm = mmap(NULL, shm_size, PROT_READ | PROT_WRITE, MAP_SHARED, shm_fd, 0); if (shm == MAP_FAILED) { - *error_code = errno; + *error_code = -1 * errno; goto CLEANUP_UNLINK; } // We have successfully mapped the memory, now initialize the region shm->magic = MAGIC; - shm->num_locks = num_locks; + shm->unused = 0; + shm->num_locks = num_bitmaps * BITMAP_SIZE; shm->num_bitmaps = num_bitmaps; - // Initialize the semaphore that protects the bitmaps. - // Initialize to value 1, as we're a mutex, and set pshared as this will be - // shared between processes in an SHM. - ret_code = sem_init(&(shm->segment_lock), true, 1); - if (ret_code < 0) { - *error_code = errno; + // Create an initializer for our pthread mutexes + ret_code = pthread_mutexattr_init(&attr); + if (ret_code != 0) { + *error_code = -1 * ret_code; goto CLEANUP_UNMAP; } + // Set mutexes to pshared - multiprocess-safe + ret_code = pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED); + if (ret_code != 0) { + *error_code = -1 * ret_code; + goto CLEANUP_FREEATTR; + } + + // Set mutexes to robust - if a process dies while holding a mutex, we'll get + // a special error code on the next attempt to lock it. + // This should prevent panicing processes from leaving the state unusable. + ret_code = pthread_mutexattr_setrobust(&attr, PTHREAD_MUTEX_ROBUST); + if (ret_code != 0) { + *error_code = -1 * ret_code; + goto CLEANUP_FREEATTR; + } + + // Initialize the mutex that protects the bitmaps using the mutex attributes + ret_code = pthread_mutex_init(&(shm->segment_lock), &attr); + if (ret_code != 0) { + *error_code = -1 * ret_code; + goto CLEANUP_FREEATTR; + } + // Initialize all bitmaps to 0 initially // And initialize all semaphores they use for (i = 0; i < num_bitmaps; i++) { shm->locks[i].bitmap = 0; for (j = 0; j < BITMAP_SIZE; j++) { - // As above, initialize to 1 to act as a mutex, and set pshared as we'll - // be living in an SHM. - ret_code = sem_init(&(shm->locks[i].locks[j]), true, 1); - if (ret_code < 0) { - *error_code = errno; - goto CLEANUP_UNMAP; + // Initialize each mutex + ret_code = pthread_mutex_init(&(shm->locks[i].locks[j]), &attr); + if (ret_code != 0) { + *error_code = -1 * ret_code; + goto CLEANUP_FREEATTR; } } } @@ -103,23 +177,33 @@ shm_struct_t *setup_lock_shm(uint32_t num_locks, int *error_code) { // Ignore errors, it's ok if we leak a single FD and this should only run once close(shm_fd); + // Destroy the pthread initializer attribute. + // Again, ignore errors, this will only run once and we might leak a tiny bit + // of memory at worst. + pthread_mutexattr_destroy(&attr); + return shm; // Cleanup after an error + CLEANUP_FREEATTR: + pthread_mutexattr_destroy(&attr); CLEANUP_UNMAP: munmap(shm, shm_size); CLEANUP_UNLINK: close(shm_fd); - shm_unlink(SHM_NAME); + shm_unlink(path); return NULL; } // Open an existing SHM segment holding libpod locks. // num_locks is the number of locks that will be configured in the SHM segment. -// num_locks must be a multiple of BITMAP_SIZE (32 by default). +// num_locks cannot be 0. +// Path is the path to the SHM segment. It must begin with a single / and +// container no other / characters, and be at most 255 characters including +// terminating NULL byte. // Returns a valid pointer on success or NULL on error. -// If an error occurs, it will be written to the int pointed to by error_code. -shm_struct_t *open_lock_shm(uint32_t num_locks, int *error_code) { +// If an error occurs, negative ERRNO values will be written to error_code. +shm_struct_t *open_lock_shm(char *path, uint32_t num_locks, int *error_code) { int shm_fd; shm_struct_t *shm; size_t shm_size; @@ -131,30 +215,34 @@ shm_struct_t *open_lock_shm(uint32_t num_locks, int *error_code) { // We need a nonzero number of locks if (num_locks == 0) { - *error_code = EINVAL; + *error_code = -1 * EINVAL; return NULL; } - // Calculate the number of bitmaps required - if (num_locks % BITMAP_SIZE != 0) { - // Number of locks not a multiple of BITMAP_SIZE - *error_code = EINVAL; + if (path == NULL) { + *error_code = -1 * EINVAL; return NULL; } + + // Calculate the number of bitmaps required num_bitmaps = num_locks / BITMAP_SIZE; + if (num_locks % BITMAP_SIZE != 0) { + num_bitmaps += 1; + } // Calculate size of the shm segment shm_size = compute_shm_size(num_bitmaps); - shm_fd = shm_open(SHM_NAME, O_RDWR, 0600); + shm_fd = shm_open(path, O_RDWR, 0600); if (shm_fd < 0) { + *error_code = -1 * errno; return NULL; } // Map the shared memory in shm = mmap(NULL, shm_size, PROT_READ | PROT_WRITE, MAP_SHARED, shm_fd, 0); if (shm == MAP_FAILED) { - *error_code = errno; + *error_code = -1 * errno; } // Ignore errors, it's ok if we leak a single FD since this only runs once @@ -167,11 +255,11 @@ shm_struct_t *open_lock_shm(uint32_t num_locks, int *error_code) { // Need to check the SHM to see if it's actually our locks if (shm->magic != MAGIC) { - *error_code = errno; + *error_code = -1 * errno; goto CLEANUP; } - if (shm->num_locks != num_locks) { - *error_code = errno; + if (shm->num_locks != (num_bitmaps * BITMAP_SIZE)) { + *error_code = -1 * errno; goto CLEANUP; } @@ -219,11 +307,9 @@ int64_t allocate_semaphore(shm_struct_t *shm) { } // Lock the semaphore controlling access to our shared memory - do { - ret_code = sem_wait(&(shm->segment_lock)); - } while(ret_code == EINTR); + ret_code = take_mutex(&(shm->segment_lock)); if (ret_code != 0) { - return -1 * errno; + return -1 * ret_code; } // Loop through our bitmaps to search for one that is not full @@ -237,8 +323,13 @@ int64_t allocate_semaphore(shm_struct_t *shm) { sem_number = (BITMAP_SIZE * i) + num_within_bitmap; // OR in the bitmap shm->locks[i].bitmap = shm->locks[i].bitmap | test_map; - // Clear the semaphore - sem_post(&(shm->segment_lock)); + + // Clear the mutex + ret_code = release_mutex(&(shm->segment_lock)); + if (ret_code != 0) { + return -1 * ret_code; + } + // Return the semaphore we've allocated return sem_number; } @@ -250,8 +341,11 @@ int64_t allocate_semaphore(shm_struct_t *shm) { } } - // Post to the semaphore to clear the lock - sem_post(&(shm->segment_lock)); + // Clear the mutex + ret_code = release_mutex(&(shm->segment_lock)); + if (ret_code != 0) { + return -1 * ret_code; + } // All bitmaps are full // We have no available semaphores, report allocation failure @@ -282,23 +376,20 @@ int32_t deallocate_semaphore(shm_struct_t *shm, uint32_t sem_index) { return -1 * EFAULT; } - test_map = 0x1; - for (i = 0; i < index_in_bitmap; i++) { - test_map = test_map << 1; - } + test_map = 0x1 << index_in_bitmap; - // Lock the semaphore controlling access to our shared memory - do { - ret_code = sem_wait(&(shm->segment_lock)); - } while(ret_code == EINTR); + // Lock the mutex controlling access to our shared memory + ret_code = take_mutex(&(shm->segment_lock)); if (ret_code != 0) { - return -1 * errno; + return -1 * ret_code; } // Check if the semaphore is allocated if ((test_map & shm->locks[bitmap_index].bitmap) == 0) { - // Post to the semaphore to clear the lock - sem_post(&(shm->segment_lock)); + ret_code = release_mutex(&(shm->segment_lock)); + if (ret_code != 0) { + return -1 * ret_code; + } return -1 * ENOENT; } @@ -308,8 +399,10 @@ int32_t deallocate_semaphore(shm_struct_t *shm, uint32_t sem_index) { test_map = ~test_map; shm->locks[bitmap_index].bitmap = shm->locks[bitmap_index].bitmap & test_map; - // Post to the semaphore to clear the lock - sem_post(&(shm->segment_lock)); + ret_code = release_mutex(&(shm->segment_lock)); + if (ret_code != 0) { + return -1 * ret_code; + } return 0; } @@ -333,15 +426,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; - // Lock the semaphore controlling access to our shared memory - do { - ret_code = sem_wait(&(shm->locks[bitmap_index].locks[index_in_bitmap])); - } while(ret_code == EINTR); - if (ret_code != 0) { - return -1 * errno; - } - - return 0; + return -1 * take_mutex(&(shm->locks[bitmap_index].locks[index_in_bitmap])); } // Unlock a given semaphore @@ -351,7 +436,6 @@ int32_t lock_semaphore(shm_struct_t *shm, uint32_t sem_index) { // Returns 0 on success, -1 on failure int32_t unlock_semaphore(shm_struct_t *shm, uint32_t sem_index) { int bitmap_index, index_in_bitmap, ret_code; - unsigned int sem_value = 0; if (shm == NULL) { return -1 * EINVAL; @@ -364,20 +448,5 @@ int32_t unlock_semaphore(shm_struct_t *shm, uint32_t sem_index) { bitmap_index = sem_index / BITMAP_SIZE; index_in_bitmap = sem_index % BITMAP_SIZE; - // Only allow a post if the semaphore is less than 1 (locked) - // This allows us to preserve mutex behavior - ret_code = sem_getvalue(&(shm->locks[bitmap_index].locks[index_in_bitmap]), &sem_value); - if (ret_code != 0) { - return -1 * errno; - } - if (sem_value >= 1) { - return -1 * EBUSY; - } - - ret_code = sem_post(&(shm->locks[bitmap_index].locks[index_in_bitmap])); - if (ret_code != 0) { - return -1 * errno; - } - - return 0; + return -1 * release_mutex(&(shm->locks[bitmap_index].locks[index_in_bitmap])); } diff --git a/libpod/lock/shm/shm_lock.go b/libpod/lock/shm/shm_lock.go index 9a9074c04..16d7f2008 100644 --- a/libpod/lock/shm/shm_lock.go +++ b/libpod/lock/shm/shm_lock.go @@ -1,47 +1,54 @@ package shm // #cgo LDFLAGS: -lrt -lpthread +// #include <stdlib.h> // #include "shm_lock.h" // const uint32_t bitmap_size_c = BITMAP_SIZE; import "C" import ( + "runtime" "syscall" + "unsafe" "github.com/pkg/errors" ) -var ( - bitmapSize uint32 = uint32(C.bitmap_size_c) +const ( + BitmapSize uint32 = uint32(C.bitmap_size_c) ) // SHMLocks is a struct enabling POSIX semaphore locking in a shared memory // segment. type SHMLocks struct { // nolint lockStruct *C.shm_struct_t - valid bool maxLocks uint32 + valid bool } // CreateSHMLock sets up a shared-memory segment holding a given number of POSIX // semaphores, and returns a struct that can be used to operate on those locks. -// numLocks must be a multiple of the lock bitmap size (by default, 32). -func CreateSHMLock(numLocks uint32) (*SHMLocks, error) { - if numLocks%bitmapSize != 0 || numLocks == 0 { - return nil, errors.Wrapf(syscall.EINVAL, "number of locks must be a multiple of %d", C.bitmap_size_c) +// numLocks must not be 0, and may be rounded up to a multiple of the bitmap +// size used by the underlying implementation. +func CreateSHMLock(path string, numLocks uint32) (*SHMLocks, error) { + if numLocks == 0 { + return nil, errors.Wrapf(syscall.EINVAL, "number of locks must greater than 0 0") } locks := new(SHMLocks) + cPath := C.CString(path) + defer C.free(unsafe.Pointer(cPath)) + var errCode C.int - lockStruct := C.setup_lock_shm(C.uint32_t(numLocks), &errCode) + lockStruct := C.setup_lock_shm(cPath, C.uint32_t(numLocks), &errCode) if lockStruct == nil { // We got a null pointer, so something errored return nil, syscall.Errno(-1 * errCode) } locks.lockStruct = lockStruct - locks.maxLocks = numLocks + locks.maxLocks = uint32(lockStruct.num_locks) locks.valid = true return locks, nil @@ -49,17 +56,19 @@ func CreateSHMLock(numLocks uint32) (*SHMLocks, error) { // OpenSHMLock opens an existing shared-memory segment holding a given number of // POSIX semaphores. numLocks must match the number of locks the shared memory -// segment was created with and be a multiple of the lock bitmap size (default -// 32). -func OpenSHMLock(numLocks uint32) (*SHMLocks, error) { - if numLocks%bitmapSize != 0 || numLocks == 0 { - return nil, errors.Wrapf(syscall.EINVAL, "number of locks must be a multiple of %d", C.bitmap_size_c) +// segment was created with. +func OpenSHMLock(path string, numLocks uint32) (*SHMLocks, error) { + if numLocks == 0 { + return nil, errors.Wrapf(syscall.EINVAL, "number of locks must greater than 0") } locks := new(SHMLocks) + cPath := C.CString(path) + defer C.free(unsafe.Pointer(cPath)) + var errCode C.int - lockStruct := C.open_lock_shm(C.uint32_t(numLocks), &errCode) + lockStruct := C.open_lock_shm(cPath, C.uint32_t(numLocks), &errCode) if lockStruct == nil { // We got a null pointer, so something errored return nil, syscall.Errno(-1 * errCode) @@ -108,6 +117,8 @@ func (locks *SHMLocks) AllocateSemaphore() (uint32, error) { return 0, errors.Wrapf(syscall.EINVAL, "locks have already been closed") } + // This returns a U64, so we have the full u32 range available for + // semaphore indexes, and can still return error codes. retCode := C.allocate_semaphore(locks.lockStruct) if retCode < 0 { // Negative errno returned @@ -154,6 +165,10 @@ func (locks *SHMLocks) LockSemaphore(sem uint32) error { return errors.Wrapf(syscall.EINVAL, "given semaphore %d is higher than maximum locks count %d", sem, locks.maxLocks) } + // For pthread mutexes, we have to guarantee lock and unlock happen in + // the same thread. + runtime.LockOSThread() + retCode := C.lock_semaphore(locks.lockStruct, C.uint32_t(sem)) if retCode < 0 { // Negative errno returned @@ -184,5 +199,12 @@ func (locks *SHMLocks) UnlockSemaphore(sem uint32) error { return syscall.Errno(-1 * retCode) } + // For pthread mutexes, we have to guarantee lock and unlock happen in + // the same thread. + // OK if we take multiple locks - UnlockOSThread() won't actually unlock + // until the number of calls equals the number of calls to + // LockOSThread() + runtime.UnlockOSThread() + return nil } diff --git a/libpod/lock/shm/shm_lock.h b/libpod/lock/shm/shm_lock.h index 18bea47e9..8e7e23fb7 100644 --- a/libpod/lock/shm/shm_lock.h +++ b/libpod/lock/shm/shm_lock.h @@ -1,14 +1,11 @@ #ifndef shm_locks_h_ #define shm_locks_h_ -#include <semaphore.h> +#include <pthread.h> #include <stdint.h> // Magic number to ensure we open the right SHM segment -#define MAGIC 0xA5A5 - -// Name of the SHM -#define SHM_NAME "/libpod_lock" +#define MAGIC 0x87D1 // Type for our bitmaps typedef uint32_t bitmap_t; @@ -18,22 +15,28 @@ typedef uint32_t bitmap_t; // Struct to hold a single bitmap and associated locks typedef struct lock_group { - bitmap_t bitmap; - sem_t locks[BITMAP_SIZE]; + bitmap_t bitmap; + pthread_mutex_t locks[BITMAP_SIZE]; } lock_group_t; -// Struct to hold our SHM locks +// Struct to hold our SHM locks. +// Unused is required to be 0 in the current implementation. If we ever make +// changes to this structure in the future, this will be repurposed as a version +// field. typedef struct shm_struct { - uint16_t magic; - sem_t segment_lock; - uint32_t num_bitmaps; - uint32_t num_locks; - lock_group_t locks[]; + uint16_t magic; + uint16_t unused; + pthread_mutex_t segment_lock; + uint32_t num_bitmaps; + uint32_t num_locks; + lock_group_t locks[]; } shm_struct_t; -size_t compute_shm_size(uint32_t num_bitmaps); -shm_struct_t *setup_lock_shm(uint32_t num_locks, int *error_code); -shm_struct_t *open_lock_shm(uint32_t num_locks, int *error_code); +static size_t compute_shm_size(uint32_t num_bitmaps); +static int take_mutex(pthread_mutex_t *mutex); +static int release_mutex(pthread_mutex_t *mutex); +shm_struct_t *setup_lock_shm(char *path, uint32_t num_locks, int *error_code); +shm_struct_t *open_lock_shm(char *path, uint32_t num_locks, int *error_code); int32_t close_lock_shm(shm_struct_t *shm); int64_t allocate_semaphore(shm_struct_t *shm); int32_t deallocate_semaphore(shm_struct_t *shm, uint32_t sem_index); diff --git a/libpod/lock/shm/shm_lock_test.go b/libpod/lock/shm/shm_lock_test.go index bc22db835..7174253d0 100644 --- a/libpod/lock/shm/shm_lock_test.go +++ b/libpod/lock/shm/shm_lock_test.go @@ -3,6 +3,7 @@ package shm import ( "fmt" "os" + "runtime" "syscall" "testing" "time" @@ -17,11 +18,13 @@ import ( // We can at least verify that the locks work within the local process. // 4 * BITMAP_SIZE to ensure we have to traverse bitmaps -const numLocks = 128 +const numLocks uint32 = 4 * BitmapSize + +const lockPath = "/libpod_test" // We need a test main to ensure that the SHM is created before the tests run func TestMain(m *testing.M) { - shmLock, err := CreateSHMLock(numLocks) + shmLock, err := CreateSHMLock(lockPath, numLocks) if err != nil { fmt.Fprintf(os.Stderr, "Error creating SHM for tests: %v\n", err) os.Exit(-1) @@ -42,19 +45,15 @@ func TestMain(m *testing.M) { } func runLockTest(t *testing.T, testFunc func(*testing.T, *SHMLocks)) { - locks, err := OpenSHMLock(numLocks) + locks, err := OpenSHMLock(lockPath, numLocks) if err != nil { t.Fatalf("Error opening locks: %v", err) } defer func() { - // Unlock and deallocate all locks - // Ignore EBUSY (lock is already unlocked) + // Deallocate all locks // Ignore ENOENT (lock is not allocated) var i uint32 for i = 0; i < numLocks; i++ { - if err := locks.UnlockSemaphore(i); err != nil && err != syscall.EBUSY { - t.Fatalf("Error unlocking semaphore %d: %v", i, err) - } if err := locks.DeallocateSemaphore(i); err != nil && err != syscall.ENOENT { t.Fatalf("Error deallocating semaphore %d: %v", i, err) } @@ -73,16 +72,22 @@ func runLockTest(t *testing.T, testFunc func(*testing.T, *SHMLocks)) { } } -// Test that creating an SHM with a bad size fails -func TestCreateNewSHMBadSize(t *testing.T) { +// Test that creating an SHM with a bad size rounds up to a good size +func TestCreateNewSHMBadSizeRoundsUp(t *testing.T) { // Odd number, not a power of 2, should never be a word size on a system - _, err := CreateSHMLock(7) - assert.Error(t, err) + lock, err := CreateSHMLock("/test1", 7) + assert.NoError(t, err) + + assert.Equal(t, lock.GetMaxLocks(), BitmapSize) + + if err := lock.Close(); err != nil { + t.Fatalf("Error closing locks: %v", err) + } } // Test that creating an SHM with 0 size fails func TestCreateNewSHMZeroSize(t *testing.T) { - _, err := CreateSHMLock(0) + _, err := CreateSHMLock("/test2", 0) assert.Error(t, err) } @@ -241,3 +246,28 @@ func TestLockSemaphoreActuallyLocks(t *testing.T) { assert.True(t, duration.Seconds() > 1.0) }) } + +// Test that locking and unlocking two semaphores succeeds +// Ensures that runtime.LockOSThread() is doing its job +func TestLockAndUnlockTwoSemaphore(t *testing.T) { + runLockTest(t, func(t *testing.T, locks *SHMLocks) { + err := locks.LockSemaphore(0) + assert.NoError(t, err) + + err = locks.LockSemaphore(1) + assert.NoError(t, err) + + err = locks.UnlockSemaphore(1) + assert.NoError(t, err) + + // Now yield scheduling + // To try and get us on another OS thread + runtime.Gosched() + + // And unlock the last semaphore + // If we are in a different OS thread, this should fail. + // However, runtime.UnlockOSThread() should guarantee we are not + err = locks.UnlockSemaphore(0) + assert.NoError(t, err) + }) +} diff --git a/libpod/lock/shm_lock_manager_linux.go b/libpod/lock/shm_lock_manager_linux.go index b1e9df12d..974431a13 100644 --- a/libpod/lock/shm_lock_manager_linux.go +++ b/libpod/lock/shm_lock_manager_linux.go @@ -3,13 +3,7 @@ package lock import ( - "fmt" - "math" - "strconv" - "syscall" - - "github.com/pkg/errors" - "github.com/projectatomic/libpod/libpod/lock/shm" + "github.com/containers/libpod/libpod/lock/shm" ) // SHMLockManager manages shared memory locks. @@ -18,8 +12,8 @@ type SHMLockManager struct { } // NewSHMLockManager makes a new SHMLockManager with the given number of locks. -func NewSHMLockManager(numLocks uint32) (Manager, error) { - locks, err := shm.CreateSHMLock(numLocks) +func NewSHMLockManager(path string, numLocks uint32) (Manager, error) { + locks, err := shm.CreateSHMLock(path, numLocks) if err != nil { return nil, err } @@ -32,8 +26,8 @@ func NewSHMLockManager(numLocks uint32) (Manager, error) { // OpenSHMLockManager opens an existing SHMLockManager with the given number of // locks. -func OpenSHMLockManager(numLocks uint32) (Manager, error) { - locks, err := shm.OpenSHMLock(numLocks) +func OpenSHMLockManager(path string, numLocks uint32) (Manager, error) { + locks, err := shm.OpenSHMLock(path, numLocks) if err != nil { return nil, err } @@ -59,27 +53,9 @@ func (m *SHMLockManager) AllocateLock() (Locker, error) { } // RetrieveLock retrieves a lock from the manager given its ID. -func (m *SHMLockManager) RetrieveLock(id string) (Locker, error) { - intID, err := strconv.ParseInt(id, 16, 64) - if err != nil { - return nil, errors.Wrapf(err, "given ID %q is not a valid SHMLockManager ID - cannot be parsed as int", id) - } - - if intID < 0 { - return nil, errors.Wrapf(syscall.EINVAL, "given ID %q is not a valid SHMLockManager ID - must be positive", id) - } - - if intID > math.MaxUint32 { - return nil, errors.Wrapf(syscall.EINVAL, "given ID %q is not a valid SHMLockManager ID - too large", id) - } - - var u32ID uint32 = uint32(intID) - if u32ID >= m.locks.GetMaxLocks() { - return nil, errors.Wrapf(syscall.EINVAL, "given ID %q is not a valid SHMLockManager ID - too large to fit", id) - } - +func (m *SHMLockManager) RetrieveLock(id uint32) (Locker, error) { lock := new(SHMLock) - lock.lockID = u32ID + lock.lockID = id lock.manager = m return lock, nil @@ -92,8 +68,8 @@ type SHMLock struct { } // ID returns the ID of the lock. -func (l *SHMLock) ID() string { - return fmt.Sprintf("%x", l.lockID) +func (l *SHMLock) ID() uint32 { + return l.lockID } // Lock acquires the lock. |