From d2b77f8b3397b3ffbbade6e04e37b291105028aa Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 18 Feb 2019 16:20:02 -0500 Subject: Do not make renumber shut down the runtime The original intent behind the requirement was to ensure that, if two SHM lock structs were open at the same time, we should not make such a runtime available to the user, and should clean it up instead. It turns out that we don't even need to open a second SHM lock struct - if we get an error mapping the first one due to a lock count mismatch, we can just delete it, and it cleans itself up when it errors. So there's no reason not to return a valid runtime. Signed-off-by: Matthew Heon --- libpod/lock/in_memory_locks.go | 4 +++- libpod/lock/lock.go | 11 +++++++++++ libpod/lock/shm_lock_manager_linux.go | 4 +++- 3 files changed, 17 insertions(+), 2 deletions(-) (limited to 'libpod/lock') diff --git a/libpod/lock/in_memory_locks.go b/libpod/lock/in_memory_locks.go index 7eb22328f..7c9605917 100644 --- a/libpod/lock/in_memory_locks.go +++ b/libpod/lock/in_memory_locks.go @@ -90,7 +90,9 @@ func (m *InMemoryManager) RetrieveLock(id uint32) (Locker, error) { return m.locks[id], nil } -// FreeAllLocks frees all locks +// FreeAllLocks frees all locks. +// This function is DANGEROUS. Please read the full comment in locks.go before +// trying to use it. func (m *InMemoryManager) FreeAllLocks() error { for _, lock := range m.locks { lock.allocated = false diff --git a/libpod/lock/lock.go b/libpod/lock/lock.go index b96393574..d6841646b 100644 --- a/libpod/lock/lock.go +++ b/libpod/lock/lock.go @@ -24,8 +24,19 @@ type Manager interface { // The underlying lock MUST be the same as another other lock with the // same UUID. RetrieveLock(id uint32) (Locker, error) + // PLEASE READ FULL DESCRIPTION BEFORE USING. // FreeAllLocks frees all allocated locks, in preparation for lock // reallocation. + // As this deallocates all presently-held locks, this can be very + // dangerous - if there are other processes running that might be + // attempting to allocate new locks and free existing locks, we may + // encounter races leading to an inconsistent state. + // (This is in addition to the fact that FreeAllLocks instantly makes + // the state inconsistent simply by using it, and requires a full + // lock renumbering to restore consistency!). + // In short, this should only be used as part of unit tests, or lock + // renumbering, where reasonable guarantees about other processes can be + // made. FreeAllLocks() error } diff --git a/libpod/lock/shm_lock_manager_linux.go b/libpod/lock/shm_lock_manager_linux.go index 187661c8b..8678958ee 100644 --- a/libpod/lock/shm_lock_manager_linux.go +++ b/libpod/lock/shm_lock_manager_linux.go @@ -71,7 +71,9 @@ func (m *SHMLockManager) RetrieveLock(id uint32) (Locker, error) { return lock, nil } -// FreeAllLocks frees all locks in the manager +// FreeAllLocks frees all locks in the manager. +// This function is DANGEROUS. Please read the full comment in locks.go before +// trying to use it. func (m *SHMLockManager) FreeAllLocks() error { return m.locks.DeallocateAllSemaphores() } -- cgit v1.2.3-54-g00ecf