remove extra state.DeepCopy from updateStateHook

All production state implementations copy the given state when storing
it to temporary or permanent storage, either explicitly with DeepCopy,
or implicitly by serializing the state. Since `updateStateHook` is
called with every state change, a second call to `DeepCopy` with that
hook doubles the overhead of `PostStateUpdate`.

Since all Hook implementations which handle `PostStateUpdate` (which is
exactly 1 in non-test code) only call state methods that eventually copy
the state, we can remove the extra copy from the `updateStateHook`
itself. The state was already locked for the duration of the
`PostStateUpdate` call previously, so no additional delay should result.

For consistency this also updates the documentation for
`PostStateUpdate` to record the new caller and callee expectations based
on the existing implementation. The Hook interface predated the
`statemgr` which provides a better interface for synchronizing state
access and therefor required extra copying at that time, while now we
can more easily declare that the method requires concurrent access be
locked for the duration of the call.

internal/terraform/hook.go

@@ -114,9 +114,11 @@ type Hook interface {
 	// function is called.
 	Stopping()
 
-	// PostStateUpdate is called each time the state is updated. It receives
-	// a deep copy of the state, which it may therefore access freely without
-	// any need for locks to protect from concurrent writes from the caller.
+	// PostStateUpdate is called each time the state is updated. The caller must
+	// coordinate a lock for the state if necessary, such that the Hook may
+	// access it freely without any need for additional locks to protect from
+	// concurrent writes. Implementations which modify or retain the state after
+	// the call has returned must copy the state.
 	PostStateUpdate(new *states.State) (HookAction, error)
 }
 

internal/terraform/update_state_hook.go

@@ -5,13 +5,10 @@ package terraform
 
 // updateStateHook calls the PostStateUpdate hook with the current state.
 func updateStateHook(ctx EvalContext) error {
-	// In principle we could grab the lock here just long enough to take a
-	// deep copy and then pass that to our hooks below, but we'll instead
-	// hold the hook for the duration to avoid the potential confusing
-	// situation of us racing to call PostStateUpdate concurrently with
-	// different state snapshots.
+	// PostStateUpdate requires that the state be locked and safe to read for
+	// the duration of the call.
 	stateSync := ctx.State()
-	state := stateSync.Lock().DeepCopy()
+	state := stateSync.Lock()
 	defer stateSync.Unlock()
 
 	// Call the hook