Skip to content

Commit 9d8a5d2

Browse files
committed
Address issue #804
Stop go routines from dangling on timeout rules.
1 parent 260a578 commit 9d8a5d2

File tree

2 files changed

+92
-5
lines changed

2 files changed

+92
-5
lines changed

motor/rule_applicator.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -808,15 +808,31 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult {
808808

809809
timeoutCtx, ruleCancel := context.WithTimeout(context.Background(), execution.Timeout)
810810
defer ruleCancel()
811-
doneChan := make(chan bool)
811+
doneChan := make(chan struct{})
812812

813-
go runRule(ctx, doneChan)
813+
localResults := []model.RuleFunctionResult{}
814+
localIgnored := []model.RuleFunctionResult{}
815+
localFixed := []model.RuleFunctionResult{}
816+
localErrs := []error{}
817+
localCtx := ctx
818+
localCtx.ruleResults = &localResults
819+
localCtx.ignoredResults = &localIgnored
820+
localCtx.fixedResults = &localFixed
821+
localCtx.errors = &localErrs
822+
823+
go runRule(localCtx, doneChan)
814824

815825
select {
816826
case <-timeoutCtx.Done():
817827
ctx.logger.Error("Rule timed out, skipping", "rule", rule.Id, "timeout", execution.Timeout)
818828
break
819829
case <-doneChan:
830+
lock.Lock()
831+
ruleResults = append(ruleResults, localResults...)
832+
ignoredResults = append(ignoredResults, localIgnored...)
833+
fixedResults = append(fixedResults, localFixed...)
834+
errs = append(errs, localErrs...)
835+
lock.Unlock()
820836
break
821837
}
822838
done <- true
@@ -872,7 +888,8 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult {
872888
}
873889
}
874890

875-
func runRule(ctx ruleContext, doneChan chan bool) {
891+
func runRule(ctx ruleContext, doneChan chan struct{}) {
892+
defer close(doneChan)
876893

877894
// Check for missing auto-fix functions when --fix is enabled
878895
if ctx.applyAutoFixes && ctx.rule.AutoFixFunction != "" {
@@ -971,7 +988,6 @@ func runRule(ctx ruleContext, doneChan chan bool) {
971988
lock.Lock()
972989
*ctx.errors = append(*ctx.errors, err)
973990
lock.Unlock()
974-
doneChan <- true
975991
return
976992
}
977993
if len(nodes) <= 0 {
@@ -999,7 +1015,6 @@ func runRule(ctx ruleContext, doneChan chan bool) {
9991015
}
10001016
}
10011017
}
1002-
doneChan <- true
10031018
}
10041019

10051020
var lock sync.Mutex

motor/rule_timeout_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package motor
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/daveshanley/vacuum/model"
8+
"github.com/daveshanley/vacuum/rulesets"
9+
"github.com/stretchr/testify/assert"
10+
"go.yaml.in/yaml/v4"
11+
)
12+
13+
type slowRule struct {
14+
sleep time.Duration
15+
}
16+
17+
func (s *slowRule) GetSchema() model.RuleFunctionSchema {
18+
return model.RuleFunctionSchema{
19+
Name: "slow",
20+
}
21+
}
22+
23+
func (s *slowRule) GetCategory() string {
24+
return model.CategoryValidation
25+
}
26+
27+
func (s *slowRule) RunRule(nodes []*yaml.Node, context model.RuleFunctionContext) []model.RuleFunctionResult {
28+
time.Sleep(s.sleep)
29+
return []model.RuleFunctionResult{
30+
{
31+
Message: "slow rule finished",
32+
},
33+
}
34+
}
35+
36+
func TestRuleTimeout_DropsResults(t *testing.T) {
37+
yml := `openapi: 3.0.0
38+
info:
39+
title: Test
40+
version: 1.0.0
41+
paths: {}`
42+
43+
rules := map[string]*model.Rule{
44+
"slow": {
45+
Id: "slow",
46+
Given: "$",
47+
RuleCategory: model.RuleCategories[model.CategoryValidation],
48+
Type: rulesets.Validation,
49+
Severity: model.SeverityError,
50+
Then: model.RuleAction{
51+
Function: "slow",
52+
},
53+
},
54+
}
55+
56+
ex := &RuleSetExecution{
57+
RuleSet: &rulesets.RuleSet{
58+
Rules: rules,
59+
},
60+
Spec: []byte(yml),
61+
Timeout: 20 * time.Millisecond,
62+
CustomFunctions: map[string]model.RuleFunction{
63+
"slow": &slowRule{sleep: 100 * time.Millisecond},
64+
},
65+
}
66+
67+
results := ApplyRulesToRuleSet(ex)
68+
assert.Len(t, results.Results, 0)
69+
70+
time.Sleep(150 * time.Millisecond)
71+
assert.Len(t, results.Results, 0)
72+
}

0 commit comments

Comments
 (0)