Skip to content

Commit

Permalink
Prevent future deadlocks
Browse files Browse the repository at this point in the history
When a (1) capped channel has more than one consumer, one of them will
effectively remain blocking as the other consumes the message from
the channel. Closing the channel unblocks any/all consumers.

Here we also unpublish previously public ModuleOperation.Done()
to further prevent deadlocks in the future, i.e. effectively
to prevent any place that schedules tasks to wait.

The only reason we keep the channel there for now is to make testing
*within the package* easier.
  • Loading branch information
radeksimko committed Nov 17, 2021
1 parent dd3a024 commit 84cee5d
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 14 deletions.
19 changes: 7 additions & 12 deletions internal/terraform/module/module_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestModuleLoader_referenceCollection(t *testing.T) {
if err != nil {
t.Fatal(err)
}
<-modOp.doneCh
<-modOp.done()

manifestOp := NewModuleOperation(modPath, op.OpTypeLoadModuleMetadata)
err = ml.EnqueueModuleOp(manifestOp)
Expand All @@ -66,17 +66,12 @@ func TestModuleLoader_referenceCollection(t *testing.T) {
}

t.Log("waiting for all operations to finish")
for i := 0; i <= 2; i++ {
select {
case <-manifestOp.doneCh:
t.Log("manifest parsed")
case <-originsOp.doneCh:
t.Log("origins collected")
case <-targetsOp.doneCh:
t.Log("targets collected")
case <-ctx.Done():
}
}
<-manifestOp.done()
t.Log("manifest parsed")
<-originsOp.done()
t.Log("origins collected")
<-targetsOp.done()
t.Log("targets collected")

mod, err := ss.Modules.ModuleByPath(modPath)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/terraform/module/module_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (mm *moduleManager) EnqueueModuleOp(modPath string, opType op.OpType, defer
modOp.Defer = deferFunc
mm.loader.EnqueueModuleOp(modOp)
if mm.syncLoading {
<-modOp.Done()
<-modOp.done()
}
return nil
}
Expand Down
3 changes: 2 additions & 1 deletion internal/terraform/module/module_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ func NewModuleOperation(modPath string, typ op.OpType) ModuleOperation {

func (mo ModuleOperation) markAsDone() {
mo.doneCh <- struct{}{}
close(mo.doneCh)
}

func (mo ModuleOperation) Done() <-chan struct{} {
func (mo ModuleOperation) done() <-chan struct{} {
return mo.doneCh
}

Expand Down

0 comments on commit 84cee5d

Please sign in to comment.