Merge pull request #24599 from hashicorp/jbardin/races

Fix races in GetVariableValue and login

command/login.go

@@ -364,7 +364,7 @@ func (c *LoginCommand) interactiveGetTokenByCode(hostname svchost.Hostname, cred
 		}),
 	}
 	go func() {
-		err = server.Serve(listener)
+		err := server.Serve(listener)
 		if err != nil && err != http.ErrServerClosed {
 			diags = diags.Append(tfdiags.Sourceless(
 				tfdiags.Error,
@@ -417,8 +417,7 @@ func (c *LoginCommand) interactiveGetTokenByCode(hostname svchost.Hostname, cred
 		return nil, diags
 	}
 
-	err = server.Close()
+	if err := server.Close(); err != nil {
-	if err != nil {
 		// The server will close soon enough when our process exits anyway,
 		// so we won't fuss about it for right now.
 		log.Printf("[WARN] login: callback server can't shut down: %s", err)

states/statemgr/filesystem_test.go

@@ -41,6 +41,7 @@ func TestFilesystemRace(t *testing.T) {
 			ls.WriteState(current)
 		}()
 	}
+	wg.Wait()
 }
 
 func TestFilesystemLocks(t *testing.T) {

terraform/eval_context_builtin.go

@@ -330,6 +330,9 @@ func (ctx *BuiltinEvalContext) SetModuleCallArguments(n addrs.ModuleCallInstance
 }
 
 func (ctx *BuiltinEvalContext) GetVariableValue(addr addrs.AbsInputVariableInstance) cty.Value {
+	ctx.VariableValuesLock.Lock()
+	defer ctx.VariableValuesLock.Unlock()
+
 	modKey := addr.Module.String()
 	modVars := ctx.VariableValues[modKey]
 	val, ok := modVars[addr.Variable.Name]

terraform/hook_test.go

@@ -1,6 +1,7 @@
 package terraform
 
 import (
+	"sync"
 	"testing"
 
 	"github.com/zclconf/go-cty/cty"
@@ -19,6 +20,7 @@ func TestNilHook_impl(t *testing.T) {
 // It is intended for testing that core code is emitting the correct hooks
 // for a given situation.
 type testHook struct {
+	mu    sync.Mutex
 	Calls []*testHookCall
 }
 
@@ -33,70 +35,98 @@ type testHookCall struct {
 }
 
 func (h *testHook) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, action plans.Action, priorState, plannedNewState cty.Value) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PreApply", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PostApply(addr addrs.AbsResourceInstance, gen states.Generation, newState cty.Value, err error) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PostApply", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PreDiff(addr addrs.AbsResourceInstance, gen states.Generation, priorState, proposedNewState cty.Value) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PreDiff", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PostDiff(addr addrs.AbsResourceInstance, gen states.Generation, action plans.Action, priorState, plannedNewState cty.Value) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PostDiff", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PreProvisionInstance(addr addrs.AbsResourceInstance, state cty.Value) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PreProvisionInstance", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PostProvisionInstance(addr addrs.AbsResourceInstance, state cty.Value) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PostProvisionInstance", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PreProvisionInstanceStep(addr addrs.AbsResourceInstance, typeName string) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PreProvisionInstanceStep", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PostProvisionInstanceStep(addr addrs.AbsResourceInstance, typeName string, err error) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PostProvisionInstanceStep", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) ProvisionOutput(addr addrs.AbsResourceInstance, typeName string, line string) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"ProvisionOutput", addr.String()})
 }
 
 func (h *testHook) PreRefresh(addr addrs.AbsResourceInstance, gen states.Generation, priorState cty.Value) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PreRefresh", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PostRefresh(addr addrs.AbsResourceInstance, gen states.Generation, priorState cty.Value, newState cty.Value) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PostRefresh", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PreImportState(addr addrs.AbsResourceInstance, importID string) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PreImportState", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PostImportState(addr addrs.AbsResourceInstance, imported []providers.ImportedResource) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PostImportState", addr.String()})
 	return HookActionContinue, nil
 }
 
 func (h *testHook) PostStateUpdate(new *states.State) (HookAction, error) {
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Calls = append(h.Calls, &testHookCall{"PostStateUpdate", ""})
 	return HookActionContinue, nil
 }