states: New SyncState type

This is a wrapper around State that is able to perform higher-level
manipulations (at the granularity of the entire state) in a
concurrency-safe manner, using the lower-level APIs exposed by State and
all of the types it contains.

The granularity of a SyncState operation roughly matches the granularity
off a state-related EvalNode in the "terraform" package, performing a
sequence of more primitive operations while guaranteeing atomicity of the
entire change.

As a compromise for convenience of usage, it's still possible to access
the individual state data objects via this API, but they are always copied
before returning to ensure that two distinct callers cannot have data
races. Callers should access the most granular object possible for their
operation.

states/module.go

@@ -146,18 +146,14 @@ func (ms *Module) SetResourceInstanceDeposed(addr addrs.ResourceInstance, key De
 	}
 }
 
-// DeposeResourceInstanceObject moves the current instance object for the
+// deposeResourceInstanceObject is the real implementation of
-// given resource instance address into the deposed set, leaving the instance
+// SyncState.DeposeResourceInstanceObject.
-// without a current object.
+func (ms *Module) deposeResourceInstanceObject(addr addrs.ResourceInstance) DeposedKey {
-//
-// The return value is the newly-allocated deposed key, or NotDeposed if the
-// given instance is already lacking a current object.
-func (ms *Module) DeposeResourceInstanceObject(addr addrs.ResourceInstance) DeposedKey {
 	is := ms.ResourceInstance(addr)
 	if is == nil {
 		return NotDeposed
 	}
-	return is.DeposeCurrentObject()
+	return is.deposeCurrentObject()
 }
 
 // SetOutputValue writes an output value into the state, overwriting any
@@ -190,3 +186,22 @@ func (ms *Module) SetLocalValue(name string, value cty.Value) {
 func (ms *Module) RemoveLocalValue(name string) {
 	delete(ms.LocalValues, name)
 }
+
+// empty returns true if the receving module state is contributing nothing
+// to the state. In other words, it returns true if the module could be
+// removed from the state altogether without changing the meaning of the state.
+//
+// In practice a module containing no objects is the same as a non-existent
+// module, and so we can opportunistically clean up once a module becomes
+// empty on the assumption that it will be re-added if needed later.
+func (ms *Module) empty() bool {
+	if ms == nil {
+		return true
+	}
+
+	// This must be updated to cover any new collections added to Module
+	// in future.
+	return (len(ms.Resources) == 0 &&
+		len(ms.OutputValues) == 0 &&
+		len(ms.LocalValues) == 0)
+}

states/resource.go

@@ -100,13 +100,11 @@ func (i *ResourceInstance) HasObjects() bool {
 	return i.Current != nil || len(i.Deposed) != 0
 }
 
-// DeposeCurrentObject moves the current generation object, if present, into
+// deposeCurrentObject is part of the real implementation of
-// the deposed set. After this method returns, the instance has no current
+// SyncState.DeposeResourceInstanceObject. The exported method uses a lock
-// object.
+// to ensure that we can safely allocate an unused deposed key without
-//
+// collision.
-// The return value is either the newly-allocated deposed key, or NotDeposed
+func (i *ResourceInstance) deposeCurrentObject() DeposedKey {
-// if the instance is already lacking a current instance object.
-func (i *ResourceInstance) DeposeCurrentObject() DeposedKey {
 	if !i.HasCurrent() {
 		return NotDeposed
 	}

states/resource_test.go

@@ -15,7 +15,7 @@ func TestResourceInstanceDeposeCurrentObject(t *testing.T) {
 	var dk DeposedKey
 
 	t.Run("first depose", func(t *testing.T) {
-		dk = is.DeposeCurrentObject() // dk is randomly-generated but should be eight characters long
+		dk = is.deposeCurrentObject() // dk is randomly-generated but should be eight characters long
 		t.Logf("deposedKey is %q", dk)
 
 		if got := is.Current; got != nil {
@@ -33,7 +33,7 @@ func TestResourceInstanceDeposeCurrentObject(t *testing.T) {
 	})
 
 	t.Run("second depose", func(t *testing.T) {
-		notDK := is.DeposeCurrentObject()
+		notDK := is.deposeCurrentObject()
 		if notDK != NotDeposed {
 			t.Errorf("got deposedKey %q; want NotDeposed", notDK)
 		}

states/state.go

@@ -14,7 +14,9 @@ import (
 // Access to State and the nested values within it is not concurrency-safe,
 // so when accessing a State object concurrently it is the caller's
 // responsibility to ensure that only one write is in progress at a time
-// and that reads only occur when no write is in progress.
+// and that reads only occur when no write is in progress. The most common
+// way to acheive this is to wrap the State in a SyncState and use the
+// higher-level atomic operations supported by that type.
 type State struct {
 	// Modules contains the state for each module. The keys in this map are
 	// an implementation detail and must not be used by outside callers.
@@ -36,6 +38,23 @@ func (s *State) Module(addr addrs.ModuleInstance) *Module {
 	return s.Modules[addr.String()]
 }
 
+// RemoveModule removes the module with the given address from the state,
+// unless it is the root module. The root module cannot be deleted, and so
+// this method will panic if that is attempted.
+//
+// Removing a module implicitly discards all of the resources, outputs and
+// local values within it, and so this should usually be done only for empty
+// modules. For callers accessing the state through a SyncState wrapper, modules
+// are automatically pruned if they are empty after one of their contained
+// elements is removed.
+func (s *State) RemoveModule(addr addrs.ModuleInstance) {
+	if addr.IsRoot() {
+		panic("attempted to remote root module")
+	}
+
+	delete(s.Modules, addr.String())
+}
+
 // RootModule is a convenient alias for Module(addrs.RootModuleInstance).
 func (s *State) RootModule() *Module {
 	return s.Modules[addrs.RootModuleInstance.String()]
@@ -94,3 +113,10 @@ func (s *State) LocalValue(addr addrs.AbsLocalValue) cty.Value {
 	}
 	return ms.LocalValues[addr.LocalValue.Name]
 }
+
+// SyncWrapper returns a SyncState object wrapping the receiver.
+func (s *State) SyncWrapper() *SyncState {
+	return &SyncState{
+		state: s,
+	}
+}

states/sync.go

@@ -0,0 +1,321 @@
+package states
+
+import (
+	"sync"
+
+	"github.com/zclconf/go-cty/cty"
+
+	"github.com/hashicorp/terraform/addrs"
+)
+
+// SyncState is a wrapper around State that provides concurrency-safe access to
+// various common operations that occur during a Terraform graph walk, or other
+// similar concurrent contexts.
+//
+// When a SyncState wrapper is in use, no concurrent direct access to the
+// underlying objects is permitted unless the caller first acquires an explicit
+// lock, using the Lock and Unlock methods. Most callers should _not_
+// explicitly lock, and should instead use the other methods of this type that
+// handle locking automatically.
+//
+// Since SyncState is able to safely consolidate multiple updates into a single
+// atomic operation, many of its methods are at a higher level than those
+// of the underlying types, and operate on the state as a whole rather than
+// on individual sub-structures of the state.
+//
+// SyncState can only protect against races within its own methods. It cannot
+// provide any guarantees about the order in which concurrent operations will
+// be processed, so callers may still need to employ higher-level techniques
+// for ensuring correct operation sequencing, such as building and walking
+// a dependency graph.
+type SyncState struct {
+	state *State
+	lock  sync.RWMutex
+}
+
+// Module returns a snapshot of the state of the module instance with the given
+// address, or nil if no such module is tracked.
+//
+// The return value is a pointer to a copy of the module state, which the
+// caller may then freely access and mutate. However, since the module state
+// tends to be a large data structure with many child objects, where possible
+// callers should prefer to use a more granular accessor to access a child
+// module directly, and thus reduce the amount of copying required.
+func (s *SyncState) Module(addr addrs.ModuleInstance) *Module {
+	s.lock.RLock()
+	ret := s.state.Module(addr).DeepCopy()
+	s.lock.RUnlock()
+	return ret
+}
+
+// OutputValue returns a snapshot of the state of the output value with the
+// given address, or nil if no such output value is tracked.
+//
+// The return value is a pointer to a copy of the output value state, which the
+// caller may then freely access and mutate.
+func (s *SyncState) OutputValue(addr addrs.AbsOutputValue) *OutputValue {
+	s.lock.RLock()
+	ret := s.state.OutputValue(addr).DeepCopy()
+	s.lock.RUnlock()
+	return ret
+}
+
+// SetOutputValue writes a given output value into the state, overwriting
+// any existing value of the same name.
+//
+// If the module containing the output is not yet tracked in state then it
+// be added as a side-effect.
+func (s *SyncState) SetOutputValue(addr addrs.AbsOutputValue, value cty.Value, sensitive bool) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ms := s.state.EnsureModule(addr.Module)
+	ms.SetOutputValue(addr.OutputValue.Name, value, sensitive)
+}
+
+// RemoveOutputValue removes the stored value for the output value with the
+// given address.
+//
+// If this results in its containing module being empty, the module will be
+// pruned from the state as a side-effect.
+func (s *SyncState) RemoveOutputValue(addr addrs.AbsOutputValue) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ms := s.state.Module(addr.Module)
+	if ms == nil {
+		return
+	}
+	ms.RemoveOutputValue(addr.OutputValue.Name)
+	s.maybePruneModule(addr.Module)
+}
+
+// LocalValue returns the current value associated with the given local value
+// address.
+func (s *SyncState) LocalValue(addr addrs.AbsLocalValue) cty.Value {
+	s.lock.RLock()
+	// cty.Value is immutable, so we don't need any extra copying here.
+	ret := s.state.LocalValue(addr)
+	s.lock.RUnlock()
+	return ret
+}
+
+// SetLocalValue writes a given output value into the state, overwriting
+// any existing value of the same name.
+//
+// If the module containing the local value is not yet tracked in state then it
+// will be added as a side-effect.
+func (s *SyncState) SetLocalValue(addr addrs.AbsLocalValue, value cty.Value) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ms := s.state.EnsureModule(addr.Module)
+	ms.SetLocalValue(addr.LocalValue.Name, value)
+}
+
+// RemoveLocalValue removes the stored value for the local value with the
+// given address.
+//
+// If this results in its containing module being empty, the module will be
+// pruned from the state as a side-effect.
+func (s *SyncState) RemoveLocalValue(addr addrs.AbsLocalValue) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ms := s.state.Module(addr.Module)
+	if ms == nil {
+		return
+	}
+	ms.RemoveLocalValue(addr.LocalValue.Name)
+	s.maybePruneModule(addr.Module)
+}
+
+// Resource returns a snapshot of the state of the resource with the given
+// address, or nil if no such resource is tracked.
+//
+// The return value is a pointer to a copy of the resource state, which the
+// caller may then freely access and mutate.
+func (s *SyncState) Resource(addr addrs.AbsResource) *Resource {
+	s.lock.RLock()
+	ret := s.state.Resource(addr).DeepCopy()
+	s.lock.RUnlock()
+	return ret
+}
+
+// ResourceInstance returns a snapshot of the state the resource instance with
+// the given address, or nil if no such instance is tracked.
+//
+// The return value is a pointer to a copy of the instance state, which the
+// caller may then freely access and mutate.
+func (s *SyncState) ResourceInstance(addr addrs.AbsResourceInstance) *ResourceInstance {
+	s.lock.RLock()
+	ret := s.state.ResourceInstance(addr).DeepCopy()
+	s.lock.RUnlock()
+	return ret
+}
+
+// ResourceInstanceObject returns a snapshot of the current instance object
+// of the given generation belonging to the instance with the given address,
+// or nil if no such object is tracked..
+//
+// The return value is a pointer to a copy of the object, which the caller may
+// then freely access and mutate.
+func (s *SyncState) ResourceInstanceObject(addr addrs.AbsResourceInstance, gen Generation) *ResourceInstanceObject {
+	s.lock.RLock()
+	defer s.lock.RUnlock()
+
+	inst := s.state.ResourceInstance(addr)
+	if inst == nil {
+		return nil
+	}
+	return inst.GetGeneration(gen)
+}
+
+// SetResourceMeta updates the resource-level metadata for the resource at
+// the given address, creating the containing module state and resource state
+// as a side-effect if not already present.
+func (s *SyncState) SetResourceMeta(addr addrs.AbsResource, eachMode EachMode, provider addrs.AbsProviderConfig) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ms := s.state.EnsureModule(addr.Module)
+	ms.SetResourceMeta(addr.Resource, eachMode, provider)
+}
+
+// RemoveResource removes the entire state for the given resource, taking with
+// it any instances associated with the resource. This should generally be
+// called only for resource objects whose instances have all been destroyed,
+// but that is not enforced by this method.
+func (s *SyncState) RemoveResource(addr addrs.AbsResource) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ms := s.state.EnsureModule(addr.Module)
+	ms.RemoveResource(addr.Resource)
+	s.maybePruneModule(addr.Module)
+}
+
+// SetResourceInstanceCurrent saves the given instance object as the current
+// generation of the resource instance with the given address, simulataneously
+// updating the recorded provider configuration address, dependencies, and
+// resource EachMode.
+//
+// Any existing current instance object for the given resource is overwritten.
+// Set obj to nil to remove the primary generation object altogether. If there
+// are no deposed objects then the instance as a whole will be removed, which
+// may in turn also remove the containing module if it becomes empty.
+//
+// The caller must ensure that the given ResourceInstanceObject is not
+// concurrently mutated during this call, but may be freely used again once
+// this function returns.
+//
+// The provider address and "each mode" are resource-wide settings and so they
+// are updated for all other instances of the same resource as a side-effect of
+// this call.
+//
+// If the containing module for this resource or the resource itself are not
+// already tracked in state then they will be added as a side-effect.
+func (s *SyncState) SetResourceInstanceCurrent(addr addrs.AbsResourceInstance, obj *ResourceInstanceObject, provider addrs.AbsProviderConfig) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ms := s.state.EnsureModule(addr.Module)
+	ms.SetResourceInstanceCurrent(addr.Resource, obj.DeepCopy(), provider)
+}
+
+// SetResourceInstanceDeposed saves the given instance object as a deposed
+// generation of the resource instance with the given address and deposed key.
+//
+// Call this method only for pre-existing deposed objects that already have
+// a known DeposedKey. For example, this method is useful if reloading objects
+// that were persisted to a state file. To mark the current object as deposed,
+// use DeposeResourceInstanceObject instead.
+//
+// The caller must ensure that the given ResourceInstanceObject is not
+// concurrently mutated during this call, but may be freely used again once
+// this function returns.
+//
+// The resource that contains the given instance must already exist in the
+// state, or this method will panic. Use Resource to check first if its
+// presence is not already guaranteed.
+//
+// Any existing current instance object for the given resource and deposed key
+// is overwritten. Set obj to nil to remove the deposed object altogether. If
+// the instance is left with no objects after this operation then it will
+// be removed from its containing resource altogether.
+//
+// If the containing module for this resource or the resource itself are not
+// already tracked in state then they will be added as a side-effect.
+func (s *SyncState) SetResourceInstanceDeposed(addr addrs.AbsResourceInstance, key DeposedKey, obj *ResourceInstanceObject) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ms := s.state.EnsureModule(addr.Module)
+	ms.SetResourceInstanceDeposed(addr.Resource, key, obj.DeepCopy())
+}
+
+// DeposeResourceInstanceObject moves the current instance object for the
+// given resource instance address into the deposed set, leaving the instance
+// without a current object.
+//
+// The return value is the newly-allocated deposed key, or NotDeposed if the
+// given instance is already lacking a current object.
+//
+// If the containing module for this resource or the resource itself are not
+// already tracked in state then there cannot be a current object for the
+// given instance, and so NotDeposed will be returned without modifying the
+// state at all.
+func (s *SyncState) DeposeResourceInstanceObject(addr addrs.AbsResourceInstance) DeposedKey {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ms := s.state.Module(addr.Module)
+	if ms == nil {
+		return NotDeposed
+	}
+
+	return ms.deposeResourceInstanceObject(addr.Resource)
+}
+
+// Lock acquires an explicit lock on the state, allowing direct read and write
+// access to the returned state object. The caller must call Unlock once
+// access is no longer needed, and then immediately discard the state pointer
+// pointer.
+//
+// Most callers should not use this. Instead, use the concurrency-safe
+// accessors and mutators provided directly on SyncState.
+func (s *SyncState) Lock() *State {
+	s.lock.Lock()
+	return s.state
+}
+
+// Unlock releases a lock previously acquired by Lock, at which point the
+// caller must cease all use of the state pointer that was returned.
+//
+// Do not call this method except to end an explicit lock acquired by
+// Lock. If a caller calls Unlock without first holding the lock, behavior
+// is undefined.
+func (s *SyncState) Unlock() {
+	s.lock.Unlock()
+}
+
+// maybePruneModule will remove a module from the state altogether if it is
+// empty, unless it's the root module which must always be present.
+//
+// This helper method is not concurrency-safe on its own, so must only be
+// called while the caller is already holding the lock for writing.
+func (s *SyncState) maybePruneModule(addr addrs.ModuleInstance) {
+	if addr.IsRoot() {
+		// We never prune the root.
+		return
+	}
+
+	ms := s.state.Module(addr)
+	if ms == nil {
+		return
+	}
+
+	if ms.empty() {
+		s.state.RemoveModule(addr)
+	}
+}