Skip to content

Commit 64288d1

Browse files
author
Victor Xian Wang
committed
Fix regexp cache race issue
The original implementation allows for read to happen without acquiring lock. This leads to data race when read and write happen concurrently. Although tempting, first implementation used sync.Map, which optimize for a map that's used as cache concurrently. Later changed it back to Mutex for support of go 1.8 as sync.Map is introduced in go 1.9 Signed-off-by: Victor Xian Wang <victor.xianwang@gmail.com>
1 parent 659e09d commit 64288d1

File tree

2 files changed

+51
-4
lines changed

2 files changed

+51
-4
lines changed

rexp.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,24 +26,24 @@ var (
2626
)
2727

2828
func compileRegexp(pattern string) (*re.Regexp, error) {
29+
cacheMutex.Lock()
30+
defer cacheMutex.Unlock()
2931
// Save repeated regexp compilation
3032
if reDict[pattern] != nil {
3133
return reDict[pattern], nil
3234
}
3335
var err error
34-
cacheMutex.Lock()
3536
reDict[pattern], err = re.Compile(pattern)
36-
cacheMutex.Unlock()
3737
return reDict[pattern], err
3838
}
3939

4040
func mustCompileRegexp(pattern string) *re.Regexp {
41+
cacheMutex.Lock()
42+
defer cacheMutex.Unlock()
4143
// Save repeated regexp compilation, with panic on error
4244
if reDict[pattern] != nil {
4345
return reDict[pattern]
4446
}
45-
defer cacheMutex.Unlock()
46-
cacheMutex.Lock()
4747
reDict[pattern] = re.MustCompile(pattern)
4848
return reDict[pattern]
4949
}

rexp_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,50 @@ func Test_mustCompileRegexp(t *testing.T) {
5757

5858
assert.Panics(t, testPanic)
5959
}
60+
61+
func TestRace_compileRegexp(t *testing.T) {
62+
vrex := new(re.Regexp)
63+
64+
patterns := []string{
65+
".*TestRegexp1.*",
66+
".*TestRegexp2.*",
67+
".*TestRegexp3.*",
68+
}
69+
70+
comp := func(pattern string) {
71+
rex, err := compileRegexp(pattern)
72+
assert.NoError(t, err)
73+
assert.NotNil(t, rex)
74+
assert.IsType(t, vrex, rex)
75+
}
76+
77+
for i := 0; i < 20; i++ {
78+
t.Run(patterns[i%3], func(t *testing.T) {
79+
t.Parallel()
80+
comp(patterns[i%3])
81+
})
82+
}
83+
}
84+
85+
func TestRace_mustCompileRegexp(t *testing.T) {
86+
vrex := new(re.Regexp)
87+
88+
patterns := []string{
89+
".*TestRegexp1.*",
90+
".*TestRegexp2.*",
91+
".*TestRegexp3.*",
92+
}
93+
94+
comp := func(pattern string) {
95+
rex := mustCompileRegexp(pattern)
96+
assert.NotNil(t, rex)
97+
assert.IsType(t, vrex, rex)
98+
}
99+
100+
for i := 0; i < 20; i++ {
101+
t.Run(patterns[i%3], func(t *testing.T) {
102+
t.Parallel()
103+
comp(patterns[i%3])
104+
})
105+
}
106+
}

0 commit comments

Comments
 (0)