Fix a panic due to unsafe concurrent map access (#5995)

State tracking for goals was implemented using a raw map,
but this was not safe for concurrent read/write access from
multiple goroutines. Switched to using a sync.Map, which
is threadsafe.
This commit is contained in:
Levi Blackstone 2020-12-22 12:04:49 -07:00 committed by GitHub
parent d0676c19a3
commit 795a11c44e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 30 additions and 6 deletions

View file

@ -3,6 +3,9 @@ CHANGELOG
## HEAD (Unreleased)
- Fix a panic due to unsafe concurrent map access.
[#5995](https://github.com/pulumi/pulumi/pull/5995)
- Fix regression in `venv` creation for python policy packs.
[#5992](https://github.com/pulumi/pulumi/pull/5992)

View file

@ -106,6 +106,22 @@ func (p PlanPendingOperationsError) Error() string {
return "one or more operations are currently pending"
}
type goalMap struct {
m sync.Map
}
func (m *goalMap) set(urn resource.URN, goal *resource.Goal) {
m.m.Store(urn, goal)
}
func (m *goalMap) get(urn resource.URN) (*resource.Goal, bool) {
g, ok := m.m.Load(urn)
if !ok {
return nil, false
}
return g.(*resource.Goal), true
}
type resourceMap struct {
m sync.Map
}
@ -144,7 +160,7 @@ type Deployment struct {
preview bool // true if this deployment is to be previewed.
depGraph *graph.DependencyGraph // the dependency graph of the old snapshot.
providers *providers.Registry // the provider registry for this deployment.
goals map[resource.URN]*resource.Goal // the set of resource goals generated by the deployment.
goals *goalMap // the set of resource goals generated by the deployment.
news *resourceMap // the set of new resources generated by the deployment.
}
@ -305,8 +321,8 @@ func NewDeployment(ctx *plugin.Context, target *Target, prev *Snapshot, source S
// Build the dependency graph for the old resources.
depGraph := graph.NewDependencyGraph(oldResources)
// Create a goals map for the deployment.
newGoals := make(map[resource.URN]*resource.Goal)
// Create a goal map for the deployment.
newGoals := &goalMap{}
// Create a resource map for the deployment.
newResources := &resourceMap{}

View file

@ -73,6 +73,9 @@ func NewImportDeployment(ctx *plugin.Context, target *Target, projectName tokens
return nil, err
}
// Create a goal map for the deployment.
newGoals := &goalMap{}
builtins := newBuiltinProvider(nil, nil)
// Create a new provider registry.
@ -87,6 +90,7 @@ func NewImportDeployment(ctx *plugin.Context, target *Target, projectName tokens
target: target,
prev: prev,
olds: olds,
goals: newGoals,
imports: imports,
isImport: true,
schemaLoader: schema.NewPluginLoader(ctx.Host),

View file

@ -293,7 +293,7 @@ func (se *stepExecutor) executeStep(workerID int, step Step) error {
}
// If this is not a resource that is managed by Pulumi, then we can ignore it.
if _, hasGoal := se.deployment.goals[newState.URN]; hasGoal {
if _, hasGoal := se.deployment.goals.get(newState.URN); hasGoal {
se.deployment.news.set(newState.URN, newState)
}
}

View file

@ -252,7 +252,7 @@ func (sg *stepGenerator) generateSteps(event RegisterResourceEvent) ([]Step, res
// Mark the URN/resource as having been seen. So we can run analyzers on all resources seen, as well as
// lookup providers for calculating replacement of resources that use the provider.
sg.deployment.goals[urn] = goal
sg.deployment.goals.set(urn, goal)
if providers.IsProviderType(goal.Type) {
sg.providers[urn] = new
}
@ -1286,7 +1286,8 @@ func (sg *stepGenerator) calculateDependentReplacements(root *resource.State) ([
func (sg *stepGenerator) AnalyzeResources() result.Result {
var resources []plugin.AnalyzerStackResource
sg.deployment.news.mapRange(func(urn resource.URN, v *resource.State) bool {
goal := sg.deployment.goals[urn]
goal, ok := sg.deployment.goals.get(urn)
contract.Assertf(ok, "failed to load goal for %s", urn)
resource := plugin.AnalyzerStackResource{
AnalyzerResource: plugin.AnalyzerResource{
URN: v.URN,