Merge pull request #8586 from hashicorp/jbardin/races

adding locks

terraform/state.go

@@ -98,8 +98,14 @@ func NewState() *State {
 // the given path. If the path is "root", for example, then children
 // returned might be "root.child", but not "root.child.grandchild".
 func (s *State) Children(path []string) []*ModuleState {
+	s.Lock()
+	defer s.Unlock()
 	// TODO: test
 
+	return s.children(path)
+}
+
+func (s *State) children(path []string) []*ModuleState {
 	result := make([]*ModuleState, 0)
 	for _, m := range s.Modules {
 		if len(m.Path) != len(path)+1 {
@@ -120,8 +126,15 @@ func (s *State) Children(path []string) []*ModuleState {
 // This should be the preferred method to add module states since it
 // allows us to optimize lookups later as well as control sorting.
 func (s *State) AddModule(path []string) *ModuleState {
+	s.Lock()
+	defer s.Unlock()
+
+	return s.addModule(path)
+}
+
+func (s *State) addModule(path []string) *ModuleState {
 	// check if the module exists first
-	m := s.ModuleByPath(path)
+	m := s.moduleByPath(path)
 	if m != nil {
 		return m
 	}
@@ -140,6 +153,13 @@ func (s *State) ModuleByPath(path []string) *ModuleState {
 	if s == nil {
 		return nil
 	}
+	s.Lock()
+	defer s.Unlock()
+
+	return s.moduleByPath(path)
+}
+
+func (s *State) moduleByPath(path []string) *ModuleState {
 	for _, mod := range s.Modules {
 		if mod.Path == nil {
 			panic("missing module path")
@@ -155,6 +175,14 @@ func (s *State) ModuleByPath(path []string) *ModuleState {
 // returning their full paths. These paths can be used with ModuleByPath
 // to return the actual state.
 func (s *State) ModuleOrphans(path []string, c *config.Config) [][]string {
+	s.Lock()
+	defer s.Unlock()
+
+	return s.moduleOrphans(path, c)
+
+}
+
+func (s *State) moduleOrphans(path []string, c *config.Config) [][]string {
 	// direct keeps track of what direct children we have both in our config
 	// and in our state. childrenKeys keeps track of what isn't an orphan.
 	direct := make(map[string]struct{})
@@ -168,7 +196,7 @@ func (s *State) ModuleOrphans(path []string, c *config.Config) [][]string {
 
 	// Go over the direct children and find any that aren't in our keys.
 	var orphans [][]string
-	for _, m := range s.Children(path) {
+	for _, m := range s.children(path) {
 		key := m.Path[len(m.Path)-1]
 
 		// Record that we found this key as a direct child. We use this
@@ -228,6 +256,8 @@ func (s *State) Empty() bool {
 	if s == nil {
 		return true
 	}
+	s.Lock()
+	defer s.Unlock()
 
 	return len(s.Modules) == 0
 }
@@ -238,6 +268,9 @@ func (s *State) IsRemote() bool {
 	if s == nil {
 		return false
 	}
+	s.Lock()
+	defer s.Unlock()
+
 	if s.Remote == nil {
 		return false
 	}
@@ -258,6 +291,9 @@ func (s *State) IsRemote() bool {
 // If this returns an error, then the user should be notified. The error
 // response will include detailed information on the nature of the error.
 func (s *State) Validate() error {
+	s.Lock()
+	defer s.Unlock()
+
 	var result error
 
 	// !!!! FOR DEVELOPERS !!!!
@@ -295,6 +331,9 @@ func (s *State) Validate() error {
 // all children as well. To check what will be deleted, use a StateFilter
 // first.
 func (s *State) Remove(addr ...string) error {
+	s.Lock()
+	defer s.Unlock()
+
 	// Filter out what we need to delete
 	filter := &StateFilter{State: s}
 	results, err := filter.Filter(addr...)
@@ -362,7 +401,7 @@ func (s *State) removeModule(path []string, v *ModuleState) {
 
 func (s *State) removeResource(path []string, v *ResourceState) {
 	// Get the module this resource lives in. If it doesn't exist, we're done.
-	mod := s.ModuleByPath(path)
+	mod := s.moduleByPath(path)
 	if mod == nil {
 		return
 	}
@@ -420,6 +459,16 @@ func (s *State) Equal(other *State) bool {
 		return s == other
 	}
 
+	s.Lock()
+	defer s.Unlock()
+	return s.equal(other)
+}
+
+func (s *State) equal(other *State) bool {
+	if s == nil || other == nil {
+		return s == other
+	}
+
 	// If the versions are different, they're certainly not equal
 	if s.Version != other.Version {
 		return false
@@ -431,7 +480,7 @@ func (s *State) Equal(other *State) bool {
 	}
 	for _, m := range s.Modules {
 		// This isn't very optimal currently but works.
-		otherM := other.ModuleByPath(m.Path)
+		otherM := other.moduleByPath(m.Path)
 		if otherM == nil {
 			return false
 		}
@@ -466,7 +515,6 @@ const (
 // An error is returned if the two states are not of the same lineage,
 // in which case the integer returned has no meaning.
 func (s *State) CompareAges(other *State) (StateAgeComparison, error) {
-
 	// nil states are "older" than actual states
 	switch {
 	case s != nil && other == nil:
@@ -483,6 +531,9 @@ func (s *State) CompareAges(other *State) (StateAgeComparison, error) {
 		)
 	}
 
+	s.Lock()
+	defer s.Unlock()
+
 	switch {
 	case s.Serial < other.Serial:
 		return StateAgeReceiverOlder, nil
@@ -496,6 +547,9 @@ func (s *State) CompareAges(other *State) (StateAgeComparison, error) {
 // SameLineage returns true only if the state given in argument belongs
 // to the same "lineage" of states as the reciever.
 func (s *State) SameLineage(other *State) bool {
+	s.Lock()
+	defer s.Unlock()
+
 	// If one of the states has no lineage then it is assumed to predate
 	// this concept, and so we'll accept it as belonging to any lineage
 	// so that a lineage string can be assigned to newer versions
@@ -527,10 +581,13 @@ func (s *State) IncrementSerialMaybe(other *State) {
 	if other == nil {
 		return
 	}
+	s.Lock()
+	defer s.Unlock()
+
 	if s.Serial > other.Serial {
 		return
 	}
-	if other.TFVersion != s.TFVersion || !s.Equal(other) {
+	if other.TFVersion != s.TFVersion || !s.equal(other) {
 		if other.Serial > s.Serial {
 			s.Serial = other.Serial
 		}
@@ -542,6 +599,9 @@ func (s *State) IncrementSerialMaybe(other *State) {
 // FromFutureTerraform checks if this state was written by a Terraform
 // version from the future.
 func (s *State) FromFutureTerraform() bool {
+	s.Lock()
+	defer s.Unlock()
+
 	// No TF version means it is certainly from the past
 	if s.TFVersion == "" {
 		return false
@@ -552,6 +612,8 @@ func (s *State) FromFutureTerraform() bool {
 }
 
 func (s *State) Init() {
+	s.Lock()
+	defer s.Unlock()
 	s.init()
 }
 
@@ -559,10 +621,10 @@ func (s *State) init() {
 	if s.Version == 0 {
 		s.Version = StateVersion
 	}
-	if s.ModuleByPath(rootModulePath) == nil {
+	if s.moduleByPath(rootModulePath) == nil {
-		s.AddModule(rootModulePath)
+		s.addModule(rootModulePath)
 	}
-	s.EnsureHasLineage()
+	s.ensureHasLineage()
 
 	for _, mod := range s.Modules {
 		mod.init()
@@ -574,6 +636,13 @@ func (s *State) init() {
 }
 
 func (s *State) EnsureHasLineage() {
+	s.Lock()
+	defer s.Unlock()
+
+	s.ensureHasLineage()
+}
+
+func (s *State) ensureHasLineage() {
 	if s.Lineage == "" {
 		s.Lineage = uuid.NewV4().String()
 		log.Printf("[DEBUG] New state was assigned lineage %q\n", s.Lineage)
@@ -585,7 +654,13 @@ func (s *State) EnsureHasLineage() {
 // AddModuleState insert this module state and override any existing ModuleState
 func (s *State) AddModuleState(mod *ModuleState) {
 	mod.init()
+	s.Lock()
+	defer s.Unlock()
 
+	s.addModuleState(mod)
+}
+
+func (s *State) addModuleState(mod *ModuleState) {
 	for i, m := range s.Modules {
 		if reflect.DeepEqual(m.Path, mod.Path) {
 			s.Modules[i] = mod
@@ -624,6 +699,8 @@ func (s *State) String() string {
 	if s == nil {
 		return "<nil>"
 	}
+	s.Lock()
+	defer s.Unlock()
 
 	var buf bytes.Buffer
 	for _, m := range s.Modules {
@@ -691,10 +768,19 @@ func (r *RemoteState) deepcopy() *RemoteState {
 }
 
 func (r *RemoteState) Empty() bool {
-	return r == nil || r.Type == ""
+	if r == nil {
+		return true
+	}
+	r.Lock()
+	defer r.Unlock()
+
+	return r.Type == ""
 }
 
 func (r *RemoteState) Equals(other *RemoteState) bool {
+	r.Lock()
+	defer r.Unlock()
+
 	if r.Type != other.Type {
 		return false
 	}
@@ -741,6 +827,8 @@ func (s *OutputState) Equal(other *OutputState) bool {
 	if s == nil || other == nil {
 		return false
 	}
+	s.Lock()
+	defer s.Unlock()
 
 	if s.Type != other.Type {
 		return false
@@ -811,6 +899,9 @@ func (s *ModuleState) Unlock() { s.mu.Unlock() }
 
 // Equal tests whether one module state is equal to another.
 func (m *ModuleState) Equal(other *ModuleState) bool {
+	m.Lock()
+	defer m.Unlock()
+
 	// Paths must be equal
 	if !reflect.DeepEqual(m.Path, other.Path) {
 		return false
@@ -859,11 +950,16 @@ func (m *ModuleState) Equal(other *ModuleState) bool {
 
 // IsRoot says whether or not this module diff is for the root module.
 func (m *ModuleState) IsRoot() bool {
+	m.Lock()
+	defer m.Unlock()
 	return reflect.DeepEqual(m.Path, rootModulePath)
 }
 
 // IsDescendent returns true if other is a descendent of this module.
 func (m *ModuleState) IsDescendent(other *ModuleState) bool {
+	m.Lock()
+	defer m.Unlock()
+
 	i := len(m.Path)
 	return len(other.Path) > i && reflect.DeepEqual(other.Path[:i], m.Path)
 }
@@ -872,6 +968,9 @@ func (m *ModuleState) IsDescendent(other *ModuleState) bool {
 // but aren't present in the configuration itself. Hence, these keys
 // represent the state of resources that are orphans.
 func (m *ModuleState) Orphans(c *config.Config) []string {
+	m.Lock()
+	defer m.Unlock()
+
 	keys := make(map[string]struct{})
 	for k, _ := range m.Resources {
 		keys[k] = struct{}{}
@@ -953,6 +1052,9 @@ func (m *ModuleState) deepcopy() *ModuleState {
 
 // prune is used to remove any resources that are no longer required
 func (m *ModuleState) prune() {
+	m.Lock()
+	defer m.Unlock()
+
 	for k, v := range m.Resources {
 		v.prune()
 
@@ -975,6 +1077,9 @@ func (m *ModuleState) sort() {
 }
 
 func (m *ModuleState) String() string {
+	m.Lock()
+	defer m.Unlock()
+
 	var buf bytes.Buffer
 
 	if len(m.Resources) == 0 {
@@ -1230,6 +1335,9 @@ func (s *ResourceState) Unlock() { s.mu.Unlock() }
 
 // Equal tests whether two ResourceStates are equal.
 func (s *ResourceState) Equal(other *ResourceState) bool {
+	s.Lock()
+	defer s.Unlock()
+
 	if s.Type != other.Type {
 		return false
 	}
@@ -1259,43 +1367,49 @@ func (s *ResourceState) Equal(other *ResourceState) bool {
 }
 
 // Taint marks a resource as tainted.
-func (r *ResourceState) Taint() {
+func (s *ResourceState) Taint() {
-	if r.Primary != nil {
+	s.Lock()
-		r.Primary.Tainted = true
+	defer s.Unlock()
+
+	if s.Primary != nil {
+		s.Primary.Tainted = true
 	}
 }
 
 // Untaint unmarks a resource as tainted.
-func (r *ResourceState) Untaint() {
+func (s *ResourceState) Untaint() {
-	if r.Primary != nil {
+	s.Lock()
-		r.Primary.Tainted = false
+	defer s.Unlock()
+
+	if s.Primary != nil {
+		s.Primary.Tainted = false
 	}
 }
 
-func (r *ResourceState) init() {
+func (s *ResourceState) init() {
-	r.Lock()
+	s.Lock()
-	defer r.Unlock()
+	defer s.Unlock()
 
-	if r.Primary == nil {
+	if s.Primary == nil {
-		r.Primary = &InstanceState{}
+		s.Primary = &InstanceState{}
 	}
-	r.Primary.init()
+	s.Primary.init()
 
-	if r.Dependencies == nil {
+	if s.Dependencies == nil {
-		r.Dependencies = []string{}
+		s.Dependencies = []string{}
 	}
 
-	if r.Deposed == nil {
+	if s.Deposed == nil {
-		r.Deposed = make([]*InstanceState, 0)
+		s.Deposed = make([]*InstanceState, 0)
 	}
 
-	for _, dep := range r.Deposed {
+	for _, dep := range s.Deposed {
 		dep.init()
 	}
 }
 
-func (r *ResourceState) deepcopy() *ResourceState {
+func (s *ResourceState) deepcopy() *ResourceState {
-	copy, err := copystructure.Config{Lock: true}.Copy(r)
+	copy, err := copystructure.Config{Lock: true}.Copy(s)
 	if err != nil {
 		panic(err)
 	}
@@ -1304,26 +1418,35 @@ func (r *ResourceState) deepcopy() *ResourceState {
 }
 
 // prune is used to remove any instances that are no longer required
-func (r *ResourceState) prune() {
+func (s *ResourceState) prune() {
-	n := len(r.Deposed)
+	s.Lock()
+	defer s.Unlock()
+
+	n := len(s.Deposed)
 	for i := 0; i < n; i++ {
-		inst := r.Deposed[i]
+		inst := s.Deposed[i]
 		if inst == nil || inst.ID == "" {
-			copy(r.Deposed[i:], r.Deposed[i+1:])
+			copy(s.Deposed[i:], s.Deposed[i+1:])
-			r.Deposed[n-1] = nil
+			s.Deposed[n-1] = nil
 			n--
 			i--
 		}
 	}
 
-	r.Deposed = r.Deposed[:n]
+	s.Deposed = s.Deposed[:n]
 }
 
-func (r *ResourceState) sort() {
+func (s *ResourceState) sort() {
-	sort.Strings(r.Dependencies)
+	s.Lock()
+	defer s.Unlock()
+
+	sort.Strings(s.Dependencies)
 }
 
 func (s *ResourceState) String() string {
+	s.Lock()
+	defer s.Unlock()
+
 	var buf bytes.Buffer
 	buf.WriteString(fmt.Sprintf("Type = %s", s.Type))
 	return buf.String()
@@ -1360,36 +1483,36 @@ type InstanceState struct {
 func (s *InstanceState) Lock()   { s.mu.Lock() }
 func (s *InstanceState) Unlock() { s.mu.Unlock() }
 
-func (i *InstanceState) init() {
+func (s *InstanceState) init() {
-	i.Lock()
+	s.Lock()
-	defer i.Unlock()
+	defer s.Unlock()
 
-	if i.Attributes == nil {
+	if s.Attributes == nil {
-		i.Attributes = make(map[string]string)
+		s.Attributes = make(map[string]string)
 	}
-	if i.Meta == nil {
+	if s.Meta == nil {
-		i.Meta = make(map[string]string)
+		s.Meta = make(map[string]string)
 	}
-	i.Ephemeral.init()
+	s.Ephemeral.init()
 }
 
 // Copy all the Fields from another InstanceState
-func (i *InstanceState) Set(from *InstanceState) {
+func (s *InstanceState) Set(from *InstanceState) {
-	i.Lock()
+	s.Lock()
-	defer i.Unlock()
+	defer s.Unlock()
 
 	from.Lock()
 	defer from.Unlock()
 
-	i.ID = from.ID
+	s.ID = from.ID
-	i.Attributes = from.Attributes
+	s.Attributes = from.Attributes
-	i.Ephemeral = from.Ephemeral
+	s.Ephemeral = from.Ephemeral
-	i.Meta = from.Meta
+	s.Meta = from.Meta
-	i.Tainted = from.Tainted
+	s.Tainted = from.Tainted
 }
 
-func (i *InstanceState) DeepCopy() *InstanceState {
+func (s *InstanceState) DeepCopy() *InstanceState {
-	copy, err := copystructure.Config{Lock: true}.Copy(i)
+	copy, err := copystructure.Config{Lock: true}.Copy(s)
 	if err != nil {
 		panic(err)
 	}
@@ -1398,7 +1521,13 @@ func (i *InstanceState) DeepCopy() *InstanceState {
 }
 
 func (s *InstanceState) Empty() bool {
-	return s == nil || s.ID == ""
+	if s == nil {
+		return true
+	}
+	s.Lock()
+	defer s.Unlock()
+
+	return s.ID == ""
 }
 
 func (s *InstanceState) Equal(other *InstanceState) bool {
@@ -1406,6 +1535,8 @@ func (s *InstanceState) Equal(other *InstanceState) bool {
 	if s == nil || other == nil {
 		return s == other
 	}
+	s.Lock()
+	defer s.Unlock()
 
 	// IDs must be equal
 	if s.ID != other.ID {
@@ -1465,6 +1596,8 @@ func (s *InstanceState) MergeDiff(d *InstanceDiff) *InstanceState {
 	result.init()
 
 	if s != nil {
+		s.Lock()
+		defer s.Unlock()
 		for k, v := range s.Attributes {
 			result.Attributes[k] = v
 		}
@@ -1487,16 +1620,19 @@ func (s *InstanceState) MergeDiff(d *InstanceDiff) *InstanceState {
 	return result
 }
 
-func (i *InstanceState) String() string {
+func (s *InstanceState) String() string {
+	s.Lock()
+	defer s.Unlock()
+
 	var buf bytes.Buffer
 
-	if i == nil || i.ID == "" {
+	if s == nil || s.ID == "" {
 		return "<not created>"
 	}
 
-	buf.WriteString(fmt.Sprintf("ID = %s\n", i.ID))
+	buf.WriteString(fmt.Sprintf("ID = %s\n", s.ID))
 
-	attributes := i.Attributes
+	attributes := s.Attributes
 	attrKeys := make([]string, 0, len(attributes))
 	for ak, _ := range attributes {
 		if ak == "id" {
@@ -1512,7 +1648,7 @@ func (i *InstanceState) String() string {
 		buf.WriteString(fmt.Sprintf("%s = %s\n", ak, av))
 	}
 
-	buf.WriteString(fmt.Sprintf("Tainted = %t\n", i.Tainted))
+	buf.WriteString(fmt.Sprintf("Tainted = %t\n", s.Tainted))
 
 	return buf.String()
 }