Skip to content

Commit

Permalink
Make mountsLock and authLock in Core configurable (#27633)
Browse files Browse the repository at this point in the history
* make mountsLock and authLock in Core configurable

* add changelog entry
  • Loading branch information
Marc Boudreau authored Jul 2, 2024
1 parent a05deb5 commit c5c185f
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 5 deletions.
3 changes: 3 additions & 0 deletions changelog/27633.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
core: make authLock and mountsLock in Core configurable via the detect_deadlocks configuration parameter.
```
8 changes: 6 additions & 2 deletions vault/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ type Core struct {

// mountsLock is used to ensure that the mounts table does not
// change underneath a calling function
mountsLock locking.DeadlockRWMutex
mountsLock locking.RWMutex

// mountMigrationTracker tracks past and ongoing remount operations
// against their migration ids
Expand All @@ -387,7 +387,7 @@ type Core struct {

// authLock is used to ensure that the auth table does not
// change underneath a calling function
authLock locking.DeadlockRWMutex
authLock locking.RWMutex

// audit is loaded after unseal since it is a protected
// configuration
Expand Down Expand Up @@ -1003,6 +1003,8 @@ func CreateCore(conf *CoreConfig) (*Core, error) {

detectDeadlocks := locking.ParseDetectDeadlockConfigParameter(conf.DetectDeadlocks)
stateLock := locking.CreateConfigurableRWMutex(detectDeadlocks, "statelock")
mountsLock := locking.CreateConfigurableRWMutex(detectDeadlocks, "mountsLock")
authLock := locking.CreateConfigurableRWMutex(detectDeadlocks, "authLock")

// Setup the core
c := &Core{
Expand All @@ -1018,6 +1020,8 @@ func CreateCore(conf *CoreConfig) (*Core, error) {
customListenerHeader: new(atomic.Value),
seal: conf.Seal,
stateLock: stateLock,
mountsLock: mountsLock,
authLock: authLock,
router: NewRouter(),
sealed: new(uint32),
sealMigrationDone: new(uint32),
Expand Down
73 changes: 73 additions & 0 deletions vault/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3438,6 +3438,79 @@ func InduceDeadlock(t *testing.T, vaultcore *Core, expected uint32) {
}
}

// TestDetectedDeadlockSetting verifies that a Core struct gets the appropriate
// locking.RWMutex implementation assigned for the stateLock, authLock, and
// mountsLock fields based on various values that could be obtained from the
// detect_deadlocks configuration parameter.
func TestDetectedDeadlockSetting(t *testing.T) {
var standardLock string = "*locking.SyncRWMutex"
var deadlockLock string = "*locking.DeadlockRWMutex"

for _, tc := range []struct {
name string
input string
expectedDetectDeadlockSlice []string
expectedStateLockImpl string
expectedAuthLockImpl string
expectedMountsLockImpl string
}{
{
name: "none",
input: "",
expectedDetectDeadlockSlice: []string{},
expectedStateLockImpl: standardLock,
expectedAuthLockImpl: standardLock,
expectedMountsLockImpl: standardLock,
},
{
name: "stateLock-only",
input: "STATELOCK",
expectedDetectDeadlockSlice: []string{"statelock"},
expectedStateLockImpl: deadlockLock,
expectedAuthLockImpl: standardLock,
expectedMountsLockImpl: standardLock,
},
{
name: "authLock-only",
input: "AuthLock",
expectedDetectDeadlockSlice: []string{"authlock"},
expectedStateLockImpl: standardLock,
expectedAuthLockImpl: deadlockLock,
expectedMountsLockImpl: standardLock,
},
{
name: "state-auth-mounts",
input: "mountsLock,AUTHlock,sTaTeLoCk",
expectedDetectDeadlockSlice: []string{"mountslock", "authlock", "statelock"},
expectedStateLockImpl: deadlockLock,
expectedAuthLockImpl: deadlockLock,
expectedMountsLockImpl: deadlockLock,
},
{
name: "stateLock-with-unrecognized",
input: "stateLock,otherLock",
expectedDetectDeadlockSlice: []string{"statelock", "otherlock"},
expectedStateLockImpl: deadlockLock,
expectedAuthLockImpl: standardLock,
expectedMountsLockImpl: standardLock,
},
} {
t.Run(tc.name, func(t *testing.T) {
core, _, _ := TestCoreUnsealedWithConfig(t, &CoreConfig{DetectDeadlocks: tc.input})

assert.ElementsMatch(t, tc.expectedDetectDeadlockSlice, core.detectDeadlocks)

stateLockImpl := fmt.Sprintf("%T", core.stateLock)
authLockImpl := fmt.Sprintf("%T", core.authLock)
mountsLockImpl := fmt.Sprintf("%T", core.mountsLock)

assert.Equal(t, tc.expectedStateLockImpl, stateLockImpl)
assert.Equal(t, tc.expectedAuthLockImpl, authLockImpl)
assert.Equal(t, tc.expectedMountsLockImpl, mountsLockImpl)
})
}
}

func TestSetSeals(t *testing.T) {
oldSeal := NewTestSeal(t, &seal.TestSealOpts{
StoredKeys: seal.StoredKeysSupportedGeneric,
Expand Down
6 changes: 3 additions & 3 deletions vault/logical_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -2225,12 +2225,12 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string,
return nil, logical.ErrReadOnly
}

var lock *locking.DeadlockRWMutex
var lock locking.RWMutex
switch {
case strings.HasPrefix(path, credentialRoutePrefix):
lock = &b.Core.authLock
lock = b.Core.authLock
default:
lock = &b.Core.mountsLock
lock = b.Core.mountsLock
}

lock.Lock()
Expand Down

0 comments on commit c5c185f

Please sign in to comment.