Review and order locked struct fields (#1493)

Signed-off-by: 1garo <alevardai427@gmail.com>
Signed-off-by: Christian Mesh <christianmesh1@gmail.com>
Co-authored-by: Christian Mesh <christianmesh1@gmail.com>

internal/backend/remote-state/consul/client.go

@@ -42,11 +42,11 @@ var lostLockErr = errors.New("consul lock was lost")
 
 // RemoteClient is a remote client that stores data in Consul.
 type RemoteClient struct {
-	Client *consulapi.Client
-	Path   string
-	GZip   bool
+	Path string
+	GZip bool
 
-	mu sync.Mutex
+	mu     sync.Mutex
+	Client *consulapi.Client
 	// lockState is true if we're using locks
 	lockState bool
 

internal/checks/state.go

@@ -35,8 +35,7 @@ import (
 // This container type is concurrency-safe for both reads and writes through
 // its various methods.
 type State struct {
-	mu sync.Mutex
-
+	mu          sync.Mutex
 	statuses    addrs.Map[addrs.ConfigCheckable, *configCheckableState]
 	failureMsgs addrs.Map[addrs.CheckRule, string]
 }

internal/cloud/state.go

@@ -47,8 +47,6 @@ const (
 // local caching so every persist will go to the remote storage and local
 // writes will go to memory.
 type State struct {
-	mu sync.Mutex
-
 	// We track two pieces of meta data in addition to the state itself:
 	//
 	// lineage - the state's unique ID
@@ -60,6 +58,7 @@ type State struct {
 	// state has changed from an existing state we read in.
 	lineage, readLineage string
 	serial, readSerial   uint64
+	mu                   sync.Mutex
 	state, readState     *states.State
 	disableLocks         bool
 	tfeClient            *tfe.Client

internal/cloud/tfe_client_mock.go

@@ -1052,7 +1052,6 @@ func (m *MockProjects) Delete(ctx context.Context, projectID string) error {
 
 type MockRuns struct {
 	sync.Mutex
-
 	client     *MockClient
 	Runs       map[string]*tfe.Run
 	workspaces map[string][]*tfe.Run

internal/command/clistate/local_state.go

@@ -22,8 +22,6 @@ import (
 
 // LocalState manages a state storage that is local to the filesystem.
 type LocalState struct {
-	mu sync.Mutex
-
 	// Path is the path to read the state from. PathOut is the path to
 	// write the state to. If PathOut is not specified, Path will be used.
 	// If PathOut already exists, it will be overwritten.
@@ -43,6 +41,7 @@ type LocalState struct {
 	// hurt to remove file we never wrote to.
 	created bool
 
+	mu        sync.Mutex
 	state     *tofu.State
 	readState *tofu.State
 	written   bool

internal/command/clistate/state.go

@@ -70,9 +70,9 @@ type Locker interface {
 }
 
 type locker struct {
-	mu      sync.Mutex
 	ctx     context.Context
 	timeout time.Duration
+	mu      sync.Mutex
 	state   statemgr.Locker
 	view    views.StateLocker
 	lockID  string

internal/command/views/hook_count.go

@@ -29,9 +29,9 @@ type countHook struct {
 	ToRemove       int
 	ToRemoveAndAdd int
 
+	sync.Mutex
 	pending map[string]plans.Action
 
-	sync.Mutex
 	tofu.NilHook
 }
 

internal/command/views/hook_json.go

@@ -39,10 +39,10 @@ type jsonHook struct {
 
 	view *JSONView
 
+	applyingLock sync.Mutex
 	// Concurrent map of resource addresses to allow the sequence of pre-apply,
 	// progress, and post-apply messages to share data about the resource
-	applying     map[string]applyProgress
-	applyingLock sync.Mutex
+	applying map[string]applyProgress
 
 	// Mockable functions for testing the progress timer goroutine
 	timeNow   func() time.Time

internal/command/views/hook_ui.go

@@ -38,13 +38,13 @@ func NewUiHook(view *View) *UiHook {
 type UiHook struct {
 	tofu.NilHook
 
-	view     *View
 	viewLock sync.Mutex
+	view     *View
 
 	periodicUiTimer time.Duration
 
-	resources     map[string]uiResourceState
 	resourcesLock sync.Mutex
+	resources     map[string]uiResourceState
 }
 
 var _ tofu.Hook = (*UiHook)(nil)

internal/dag/walk.go

@@ -49,15 +49,15 @@ type Walker struct {
 	// changeLock must be held to modify any of the fields below. Only Update
 	// should modify these fields. Modifying them outside of Update can cause
 	// serious problems.
-	changeLock sync.Mutex
-	vertices   Set
-	edges      Set
-	vertexMap  map[Vertex]*walkerVertex
+	changeLock      sync.Mutex
+	vertices, edges Set
+	vertexMap       map[Vertex]*walkerVertex
 
 	// wait is done when all vertices have executed. It may become "undone"
 	// if new vertices are added.
 	wait sync.WaitGroup
 
+	diagsLock sync.Mutex
 	// diagsMap contains the diagnostics recorded so far for execution,
 	// and upstreamFailed contains all the vertices whose problems were
 	// caused by upstream failures, and thus whose diagnostics should be
@@ -66,7 +66,6 @@ type Walker struct {
 	// Readers and writers of either map must hold diagsLock.
 	diagsMap       map[Vertex]tfdiags.Diagnostics
 	upstreamFailed map[Vertex]struct{}
-	diagsLock      sync.Mutex
 }
 
 func (w *Walker) init() {
@@ -92,6 +91,7 @@ type walkerVertex struct {
 	DoneCh   chan struct{}
 	CancelCh chan struct{}
 
+	DepsLock sync.Mutex
 	// Dependency information. Any changes to any of these fields requires
 	// holding DepsLock.
 	//
@@ -102,7 +102,6 @@ type walkerVertex struct {
 	// DepsUpdateCh is closed when there is a new DepsCh set.
 	DepsCh       chan bool
 	DepsUpdateCh chan struct{}
-	DepsLock     sync.Mutex
 
 	// Below is not safe to read/write in parallel. This behavior is
 	// enforced by changes only happening in Update. Nothing else should

internal/getproviders/memoize_source.go

@@ -25,9 +25,9 @@ import (
 // sequentially.
 type MemoizeSource struct {
 	underlying        Source
+	mu                sync.Mutex
 	availableVersions map[addrs.Provider]memoizeAvailableVersionsRet
 	packageMetas      map[memoizePackageMetaCall]memoizePackageMetaRet
-	mu                sync.Mutex
 }
 
 type memoizeAvailableVersionsRet struct {

internal/lang/scope.go

@@ -56,8 +56,8 @@ type Scope struct {
 	// then differ during apply.
 	PureOnly bool
 
-	funcs     map[string]function.Function
 	funcsLock sync.Mutex
+	funcs     map[string]function.Function
 
 	// activeExperiments is an optional set of experiments that should be
 	// considered as active in the module that this scope will be used for.

internal/legacy/helper/schema/field_writer_map.go

@@ -17,9 +17,8 @@ import (
 
 // MapFieldWriter writes data into a single map[string]string structure.
 type MapFieldWriter struct {
-	Schema map[string]*Schema
-
 	lock   sync.Mutex
+	Schema map[string]*Schema
 	result map[string]string
 }
 

internal/legacy/helper/schema/resource_diff.go

@@ -21,12 +21,11 @@ import (
 type newValueWriter struct {
 	*MapFieldWriter
 
-	// A list of keys that should be marked as computed.
-	computedKeys map[string]bool
-
 	// A lock to prevent races on writes. The underlying writer will have one as
 	// well - this is for computed keys.
 	lock sync.Mutex
+	// A list of keys that should be marked as computed.
+	computedKeys map[string]bool
 
 	// To be used with init.
 	once sync.Once

internal/legacy/tofu/state.go

@@ -112,10 +112,9 @@ type State struct {
 	// configuration.
 	Backend *BackendState `json:"backend,omitempty"`
 
+	mu sync.Mutex
 	// Modules contains all the modules in a breadth-first order
 	Modules []*ModuleState `json:"modules"`
-
-	mu sync.Mutex
 }
 
 func (s *State) Lock()   { s.mu.Lock() }
@@ -837,11 +836,10 @@ type RemoteState struct {
 	// Type controls the client we use for the remote state
 	Type string `json:"type"`
 
+	mu sync.Mutex
 	// Config is used to store arbitrary configuration that
 	// is type specific
 	Config map[string]string `json:"config"`
-
-	mu sync.Mutex
 }
 
 func (s *RemoteState) Lock()   { s.mu.Lock() }
@@ -1621,6 +1619,7 @@ type InstanceState struct {
 	// and is only meant as a lookup mechanism for the providers.
 	ID string `json:"id"`
 
+	mu sync.Mutex
 	// Attributes are basic information about the resource. Any keys here
 	// are accessible in variable format within OpenTofu configurations:
 	// ${resourcetype.name.attribute}.
@@ -1641,8 +1640,6 @@ type InstanceState struct {
 
 	// Tainted is used to mark a resource for recreation.
 	Tainted bool `json:"tainted"`
-
-	mu sync.Mutex
 }
 
 func (s *InstanceState) Lock()   { s.mu.Lock() }

internal/logging/panic.go

@@ -127,7 +127,6 @@ func PluginPanics() []string {
 // happened when a plugin suddenly terminates.
 type panicRecorder struct {
 	sync.Mutex
-
 	// panics maps the plugin name to the panic output lines received from
 	// the logger.
 	panics map[string][]string

internal/plugin/grpc_provider.go

@@ -70,9 +70,9 @@ type GRPCProvider struct {
 	// plugin process ends.
 	ctx context.Context
 
+	mu sync.Mutex
 	// schema stores the schema for this provider. This is used to properly
 	// serialize the requests for schemas.
-	mu     sync.Mutex
 	schema providers.GetProviderSchemaResponse
 }
 

internal/plugin/grpc_provisioner.go

@@ -48,8 +48,8 @@ type GRPCProvisioner struct {
 	client proto.ProvisionerClient
 	ctx    context.Context
 
+	mu sync.Mutex
 	// Cache the schema since we need it for serialization in each method call.
-	mu     sync.Mutex
 	schema *configschema.Block
 }
 

internal/plugin6/grpc_provider.go

@@ -70,9 +70,9 @@ type GRPCProvider struct {
 	// plugin process ends.
 	ctx context.Context
 
+	mu sync.Mutex
 	// schema stores the schema for this provider. This is used to properly
 	// serialize the requests for schemas.
-	mu     sync.Mutex
 	schema providers.GetProviderSchemaResponse
 }
 

internal/states/remote/state.go

@@ -26,8 +26,6 @@ import (
 // local caching so every persist will go to the remote storage and local
 // writes will go to memory.
 type State struct {
-	mu sync.Mutex
-
 	Client Client
 
 	encryption encryption.StateEncryption
@@ -43,6 +41,7 @@ type State struct {
 	// state has changed from an existing state we read in.
 	lineage, readLineage string
 	serial, readSerial   uint64
+	mu                   sync.Mutex
 	state, readState     *states.State
 	disableLocks         bool
 

internal/states/statemgr/filesystem.go

@@ -29,8 +29,6 @@ import (
 //
 // The transient storage for Filesystem is always in-memory.
 type Filesystem struct {
-	mu sync.Mutex
-
 	// path is the location where a file will be created or replaced for
 	// each persistent snapshot.
 	path string
@@ -59,10 +57,10 @@ type Filesystem struct {
 	// hurt to remove file we never wrote to.
 	created bool
 
-	file          *statefile.File
-	readFile      *statefile.File
-	backupFile    *statefile.File
-	writtenBackup bool
+	mu             sync.Mutex
+	file, readFile *statefile.File
+	backupFile     *statefile.File
+	writtenBackup  bool
 
 	encryption encryption.StateEncryption
 }

internal/tofu/context.go

@@ -85,8 +85,8 @@ type Context struct {
 	sh      *stopHook
 	uiInput UIInput
 
-	l                   sync.Mutex // Lock acquired during any task
 	parallelSem         Semaphore
+	l                   sync.Mutex // Lock acquired during any task
 	providerInputConfig map[string]map[string]cty.Value
 	runCond             *sync.Cond
 	runContext          context.Context

internal/tofu/eval_context_builtin.go

@@ -52,26 +52,29 @@ type BuiltinEvalContext struct {
 	// eval context.
 	Evaluator *Evaluator
 
+	VariableValuesLock *sync.Mutex
 	// VariableValues contains the variable values across all modules. This
 	// structure is shared across the entire containing context, and so it
 	// may be accessed only when holding VariableValuesLock.
 	// The keys of the first level of VariableValues are the string
 	// representations of addrs.ModuleInstance values. The second-level keys
 	// are variable names within each module instance.
-	VariableValues     map[string]map[string]cty.Value
-	VariableValuesLock *sync.Mutex
+	VariableValues map[string]map[string]cty.Value
 
 	// Plugins is a library of plugin components (providers and provisioners)
 	// available for use during a graph walk.
 	Plugins *contextPlugins
 
-	Hooks                 []Hook
-	InputValue            UIInput
-	ProviderCache         map[string]providers.Interface
-	ProviderInputConfig   map[string]map[string]cty.Value
-	ProviderLock          *sync.Mutex
-	ProvisionerCache      map[string]provisioners.Interface
-	ProvisionerLock       *sync.Mutex
+	Hooks      []Hook
+	InputValue UIInput
+
+	ProviderLock        *sync.Mutex
+	ProviderCache       map[string]providers.Interface
+	ProviderInputConfig map[string]map[string]cty.Value
+
+	ProvisionerLock  *sync.Mutex
+	ProvisionerCache map[string]provisioners.Interface
+
 	ChangesValue          *plans.ChangesSync
 	StateValue            *states.SyncState
 	ChecksValue           *checks.State

internal/tofu/evaluate.go

@@ -41,6 +41,7 @@ type Evaluator struct {
 	// Config is the root node in the configuration tree.
 	Config *configs.Config
 
+	VariableValuesLock *sync.Mutex
 	// VariableValues is a map from variable names to their associated values,
 	// within the module indicated by ModulePath. VariableValues is modified
 	// concurrently, and so it must be accessed only while holding
@@ -48,8 +49,7 @@ type Evaluator struct {
 	//
 	// The first map level is string representations of addr.ModuleInstance
 	// values, while the second level is variable names.
-	VariableValues     map[string]map[string]cty.Value
-	VariableValuesLock *sync.Mutex
+	VariableValues map[string]map[string]cty.Value
 
 	// Plugins is the library of available plugin components (providers and
 	// provisioners) that we have available to help us evaluate expressions

internal/tofu/graph_walk_context.go

@@ -51,15 +51,18 @@ type ContextGraphWalker struct {
 	// is in progress.
 	NonFatalDiagnostics tfdiags.Diagnostics
 
-	once               sync.Once
-	contexts           map[string]*BuiltinEvalContext
-	contextLock        sync.Mutex
-	variableValues     map[string]map[string]cty.Value
+	once        sync.Once
+	contextLock sync.Mutex
+	contexts    map[string]*BuiltinEvalContext
+
 	variableValuesLock sync.Mutex
-	providerCache      map[string]providers.Interface
-	providerLock       sync.Mutex
-	provisionerCache   map[string]provisioners.Interface
-	provisionerLock    sync.Mutex
+	variableValues     map[string]map[string]cty.Value
+
+	providerLock  sync.Mutex
+	providerCache map[string]providers.Interface
+
+	provisionerLock  sync.Mutex
+	provisionerCache map[string]provisioners.Interface
 }
 
 func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) EvalContext {

internal/tofu/opentf_test.go

@@ -250,11 +250,10 @@ type HookRecordApplyOrder struct {
 
 	Active bool
 
+	l      sync.Mutex
 	IDs    []string
 	States []cty.Value
 	Diffs  []*plans.Change
-
-	l sync.Mutex
 }
 
 func (h *HookRecordApplyOrder) PreApply(addr addrs.AbsResourceInstance, gen states.Generation, action plans.Action, priorState, plannedNewState cty.Value) (HookAction, error) {