vault: fix deadlock in SetConfig

This seems to be the minimum viable patch for fixing a deadlock between
establishConnection and SetConfig.

SetConfig calls tomb.Kill+tomb.Wait while holding v.lock.
establishConnection needs to acquire v.lock to exit but SetConfig is
holding v.lock until tomb.Wait exits. tomb.Wait can't exit until
establishConnect does!

```
  SetConfig -> tomb.Wait
     ^              |
     |              v
  v.lock <- establishConnection
```
This commit is contained in:
Michael Schurter
2019-08-06 10:40:14 -07:00
parent 692cd9c19e
commit 7e08a2fe45
2 changed files with 41 additions and 4 deletions

View File

@@ -341,12 +341,17 @@ func (v *vaultClient) SetConfig(config *config.VaultConfig) error {
// Kill any background routines
if v.running {
// Stop accepting any new request
v.connEstablished = false
// Kill any background routine and create a new tomb
// Kill any background routine
v.tomb.Kill(nil)
// Locking around tomb.Wait can deadlock with
// establishConnection exiting, so we must unlock here.
v.l.Unlock()
v.tomb.Wait()
v.l.Lock()
// Stop accepting any new requests
v.connEstablished = false
v.tomb = &tomb.Tomb{}
v.running = false
}

View File

@@ -519,6 +519,38 @@ func TestVaultClient_SetConfig(t *testing.T) {
}
}
// TestVaultClient_SetConfig_Deadlock asserts that calling SetConfig
// concurrently with establishConnection does not deadlock.
func TestVaultClient_SetConfig_Deadlock(t *testing.T) {
t.Parallel()
v := testutil.NewTestVault(t)
defer v.Stop()
v2 := testutil.NewTestVault(t)
defer v2.Stop()
// Set the configs token in a new test role
v2.Config.Token = defaultTestVaultWhitelistRoleAndToken(v2, t, 20)
logger := testlog.HCLogger(t)
client, err := NewVaultClient(v.Config, logger, nil)
if err != nil {
t.Fatalf("failed to build vault client: %v", err)
}
defer client.Stop()
for i := 0; i < 100; i++ {
// Alternate configs to cause updates
conf := v.Config
if i%2 == 0 {
conf = v2.Config
}
if err := client.SetConfig(conf); err != nil {
t.Fatalf("SetConfig failed: %v", err)
}
}
}
// Test that we can disable vault
func TestVaultClient_SetConfig_Disable(t *testing.T) {
t.Parallel()