From 695a5fe27dff68a1cb1e113cb12a3e24c33b8e4d Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 8 Apr 2020 09:59:27 -0400 Subject: [PATCH 1/4] lock was missing in the call to GetVariableValue --- terraform/eval_context_builtin.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 1f890bb91a..4c2ff128c2 100644 --- a/terraform/eval_context_builtin.go +++ b/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] From 85593b432e0ddca6d8343a1f8cad3ba488d2e269 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 8 Apr 2020 10:02:43 -0400 Subject: [PATCH 2/4] add locks to testHook --- terraform/hook_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/terraform/hook_test.go b/terraform/hook_test.go index 616ab3119f..6b486f1f49 100644 --- a/terraform/hook_test.go +++ b/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 } From b84e6f7f9597329ad4ae45389f029d437b84fbd0 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 8 Apr 2020 10:12:46 -0400 Subject: [PATCH 3/4] remove race from closed-over err variable --- command/login.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/command/login.go b/command/login.go index 249ff4278c..323dd7802c 100644 --- a/command/login.go +++ b/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 != nil { + if err := server.Close(); 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) From 3ee9cf49ce46f7b939dca53f3c703a6be2211791 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 8 Apr 2020 10:55:20 -0400 Subject: [PATCH 4/4] missing wg.Wait in concurrent test --- states/statemgr/filesystem_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/states/statemgr/filesystem_test.go b/states/statemgr/filesystem_test.go index 912d6ce45e..eb2179a36f 100644 --- a/states/statemgr/filesystem_test.go +++ b/states/statemgr/filesystem_test.go @@ -41,6 +41,7 @@ func TestFilesystemRace(t *testing.T) { ls.WriteState(current) }() } + wg.Wait() } func TestFilesystemLocks(t *testing.T) {