stackeval: Don't share objects between AsyncTask closures

This is, of course, the classic Go for loop hazard where a closure in the
loop captures the symbol itself rather than the value of the symbol, and
the symbol gets modified directly on each iteration so all of the closures
end up referring to whichever value came last.

This is just an ugly tactical fix for that intentionally not changing the
existing code too much. Hopefully in a future commit we'll rearrange this
a little so that it's harder to accidentally reintroduce this bug later,
and we should also add some more complex test cases with multiple embedded
stack calls and multiple component calls.
pull/34738/head
Martin Atkins 3 years ago
parent d1eaf199c9
commit 7c1dbc9061

@ -136,7 +136,14 @@ func (m *Main) walkPlanChanges(ctx context.Context, walk *planWalk, stack *Stack
// we can explore downstream stacks concurrently with this one. Each
// stack call can represent zero or more child stacks that we'll analyze
// by recursive calls to this function.
for _, call := range stack.EmbeddedStackCalls(ctx) {
for _, obj := range stack.EmbeddedStackCalls(ctx) {
// This must be a local variable inside the loop and _not_ a
// loop iterator variable because otherwise the function below
// will capture the same variable on every iteration, rather
// than a separate value each time.
call := obj
obj = nil // DO NOT use obj in the rest of this loop
m.walkPlanValidateConfig(ctx, walk, call.Config(ctx))
// We need to perform the whole expansion in an overall async task
@ -160,14 +167,21 @@ func (m *Main) walkPlanChanges(ctx context.Context, walk *planWalk, stack *Stack
// We also need to plan all of the other declarations in the current stack.
for _, obj := range stack.Components(ctx) {
m.walkPlanValidateConfig(ctx, walk, obj.Config(ctx))
m.walkPlanObjectChanges(ctx, walk, obj)
// This must be a local variable inside the loop and _not_ a
// loop iterator variable because otherwise the function below
// will capture the same variable on every iteration, rather
// than a separate value each time.
component := obj
obj = nil // DO NOT use obj in the rest of this loop
m.walkPlanValidateConfig(ctx, walk, component.Config(ctx))
m.walkPlanObjectChanges(ctx, walk, component)
// We need to perform the instance expansion in an overall async task
// because it involves potentially evaluating a for_each expression.
// and that might depend on data from elsewhere in the same stack.
walk.AsyncTask(ctx, func(ctx context.Context) {
insts := obj.Instances(ctx, PlanPhase)
insts := component.Instances(ctx, PlanPhase)
for _, inst := range insts {
m.walkPlanObjectChanges(ctx, walk, inst)
}

Loading…
Cancel
Save