Skip to content

Commit

Permalink
aghnet: fix races
Browse files Browse the repository at this point in the history
  • Loading branch information
@EugeneOne1
EugeneOne1 committed Jan 10, 2022
1 parent fd66191 commit 9f341eb
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 121 deletions.
12 changes: 6 additions & 6 deletions internal/aghnet/hostscontainer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,9 @@ func pathsToPatterns(fsys fs.FS, paths []string) (patterns []string, err error)
return patterns, nil
}

// handleEvents concurrently handles the events. It closes the update channel
// of HostsContainer when finishes. Used to be called within a goroutine.
// handleEvents concurrently handles the file system events. It closes the
// update channel of HostsContainer when finishes. It's used to be called
// within a separate goroutine.
func (hc *HostsContainer) handleEvents() {
defer log.OnPanic(fmt.Sprintf("%s: handling events", hostsContainerPref))

Expand Down Expand Up @@ -292,10 +293,9 @@ func (hc *HostsContainer) newHostsParser() (hp *hostsParser) {

return &hostsParser{
rulesBuilder: &strings.Builder{},
// For A/AAAA and PTRs.
rules: make([]ipRules, 0, lastLen),
cnameSet: stringutil.NewSet(),
table: netutil.NewIPMap(lastLen),
rules: make([]ipRules, 0, lastLen),
cnameSet: stringutil.NewSet(),
table: netutil.NewIPMap(lastLen),
}
}

Expand Down
178 changes: 63 additions & 115 deletions internal/aghnet/hostscontainer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,24 +130,13 @@ func TestNewHostsContainer(t *testing.T) {
})
}

func TestHostsContainer_Refresh(t *testing.T) {
knownIP := net.IP{127, 0, 0, 1}
func TestHostsContainer_refresh(t *testing.T) {
// TODO(e.burkov): Test the case with no actual updates.

const knownHost = "localhost"
const knownAlias = "hocallost"
ip := net.IP{127, 0, 0, 1}
ipStr := ip.String()

const dirname = "dir"
const filename1 = "file1"
const filename2 = "file2"

p1 := path.Join(dirname, filename1)
p2 := path.Join(dirname, filename2)

testFS := fstest.MapFS{
p1: &fstest.MapFile{
Data: []byte(strings.Join([]string{knownIP.String(), knownHost}, sp) + nl),
},
}
testFS := fstest.MapFS{"dir/file1": &fstest.MapFile{Data: []byte(ipStr + ` hostname` + nl)}}

// event is a convenient alias for an empty struct{} to emit test events.
type event = struct{}
Expand All @@ -158,14 +147,14 @@ func TestHostsContainer_Refresh(t *testing.T) {
w := &aghtest.FSWatcher{
OnEvents: func() (e <-chan event) { return eventsCh },
OnAdd: func(name string) (err error) {
assert.Equal(t, dirname, name)
assert.Equal(t, "dir", name)

return nil
},
OnClose: func() (err error) { panic("not implemented") },
}

hc, err := NewHostsContainer(0, testFS, w, dirname)
hc, err := NewHostsContainer(0, testFS, w, "dir")
require.NoError(t, err)

checkRefresh := func(t *testing.T, wantHosts Hosts) {
Expand All @@ -175,7 +164,7 @@ func TestHostsContainer_Refresh(t *testing.T) {

assert.Equal(t, 1, upd.Len())

v, ok := upd.Get(knownIP)
v, ok := upd.Get(ip)
require.True(t, ok)

var hosts *Hosts
Expand All @@ -187,125 +176,88 @@ func TestHostsContainer_Refresh(t *testing.T) {
}

t.Run("initial_refresh", func(t *testing.T) {
checkRefresh(t, Hosts{Main: knownHost})
checkRefresh(t, Hosts{Main: "hostname"})
})

t.Run("second_refresh", func(t *testing.T) {
testFS[p2] = &fstest.MapFile{
Data: []byte(strings.Join([]string{knownIP.String(), knownAlias}, sp) + nl),
}
testFS["dir/file2"] = &fstest.MapFile{Data: []byte(ipStr + ` alias` + nl)}
eventsCh <- event{}
checkRefresh(t, Hosts{Main: knownHost, Aliases: stringutil.NewSet(knownAlias)})
})

t.Run("no_changes_refresh", func(t *testing.T) {
eventsCh <- event{}
assert.Never(t, func() bool {
return len(hc.Upd()) > 0
}, 3*time.Second, 1*time.Second)
checkRefresh(t, Hosts{Main: "hostname", Aliases: stringutil.NewSet("alias")})
})

t.Run("double_refresh", func(t *testing.T) {
testFS[p1] = &fstest.MapFile{
Data: []byte(strings.Join([]string{knownIP.String(), knownAlias}, sp) + nl),
}
// Make a change once.
testFS["dir/file1"] = &fstest.MapFile{Data: []byte(ipStr + ` alias` + nl)}
eventsCh <- event{}

// Require the changes are written.
require.Eventually(t, func() bool {
res, ok := hc.MatchRequest(urlfilter.DNSRequest{
Hostname: knownHost,
Hostname: "hostname",
DNSType: dns.TypeA,
})

return !ok && res.DNSRewrites() == nil
}, 5*time.Second, 1*time.Second)
}, 5*time.Second, time.Second/2)

testFS[p2] = &fstest.MapFile{
Data: []byte(strings.Join([]string{knownIP.String(), knownHost}, sp) + nl),
}
// Make a change again.
testFS["dir/file2"] = &fstest.MapFile{Data: []byte(ipStr + ` hostname` + nl)}
eventsCh <- event{}

// Require the changes are written.
require.Eventually(t, func() bool {
res, ok := hc.MatchRequest(urlfilter.DNSRequest{
Hostname: knownHost,
Hostname: "hostname",
DNSType: dns.TypeA,
})

return !ok && res.DNSRewrites() != nil
}, 5*time.Second, 1*time.Second)
}, 5*time.Second, time.Second/2)

assert.Len(t, hc.Upd(), 1)
})
}

func TestHostsContainer_PathsToPatterns(t *testing.T) {
const (
dir0 = "dir"
dir1 = "dir_1"
fn1 = "file_1"
fn2 = "file_2"
fn3 = "file_3"
fn4 = "file_4"
)

fp1 := path.Join(dir0, fn1)
fp2 := path.Join(dir0, fn2)
fp3 := path.Join(dir0, dir1, fn3)

gsfs := fstest.MapFS{
fp1: &fstest.MapFile{Data: []byte{1}},
fp2: &fstest.MapFile{Data: []byte{2}},
fp3: &fstest.MapFile{Data: []byte{3}},
"dir_0/file_1": &fstest.MapFile{Data: []byte{1}},
"dir_0/file_2": &fstest.MapFile{Data: []byte{2}},
"dir_0/dir_1/file_3": &fstest.MapFile{Data: []byte{3}},
}

testCases := []struct {
name string
wantErr error
want []string
paths []string
name string
paths []string
want []string
}{{
name: "no_paths",
wantErr: nil,
want: nil,
paths: nil,
name: "no_paths",
paths: nil,
want: nil,
}, {
name: "single_file",
wantErr: nil,
want: []string{fp1},
paths: []string{fp1},
name: "single_file",
paths: []string{"dir_0/file_1"},
want: []string{"dir_0/file_1"},
}, {
name: "several_files",
wantErr: nil,
want: []string{fp1, fp2},
paths: []string{fp1, fp2},
name: "several_files",
paths: []string{"dir_0/file_1", "dir_0/file_2"},
want: []string{"dir_0/file_1", "dir_0/file_2"},
}, {
name: "whole_dir",
wantErr: nil,
want: []string{path.Join(dir0, "*")},
paths: []string{dir0},
name: "whole_dir",
paths: []string{"dir_0"},
want: []string{"dir_0/*"},
}, {
name: "file_and_dir",
wantErr: nil,
want: []string{fp1, path.Join(dir0, dir1, "*")},
paths: []string{fp1, path.Join(dir0, dir1)},
name: "file_and_dir",
paths: []string{"dir_0/file_1", "dir_0/dir_1"},
want: []string{"dir_0/file_1", "dir_0/dir_1/*"},
}, {
name: "non-existing",
wantErr: nil,
want: nil,
paths: []string{path.Join(dir0, "file_3")},
name: "non-existing",
paths: []string{path.Join("dir_0", "file_3")},
want: nil,
}}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
patterns, err := pathsToPatterns(gsfs, tc.paths)
if tc.wantErr != nil {
assert.ErrorIs(t, err, tc.wantErr)

return
}

require.NoError(t, err)

assert.Equal(t, tc.want, patterns)
Expand Down Expand Up @@ -522,12 +474,8 @@ func TestHostsContainer(t *testing.T) {
}

func TestUniqueRules_ParseLine(t *testing.T) {
const (
hostname = "localhost"
alias = "hocallost"
)

knownIP := net.IP{127, 0, 0, 1}
ip := net.IP{127, 0, 0, 1}
ipStr := ip.String()

testCases := []struct {
name string
Expand All @@ -536,39 +484,39 @@ func TestUniqueRules_ParseLine(t *testing.T) {
wantHosts []string
}{{
name: "simple",
line: strings.Join([]string{knownIP.String(), hostname}, sp),
wantIP: knownIP,
wantHosts: []string{"localhost"},
line: ipStr + ` hostname`,
wantIP: ip,
wantHosts: []string{"hostname"},
}, {
name: "aliases",
line: strings.Join([]string{knownIP.String(), hostname, alias}, sp),
wantIP: knownIP,
wantHosts: []string{"localhost", "hocallost"},
line: ipStr + ` hostname alias`,
wantIP: ip,
wantHosts: []string{"hostname", "alias"},
}, {
name: "invalid_line",
line: knownIP.String(),
line: ipStr,
wantIP: nil,
wantHosts: nil,
}, {
name: "invalid_line_hostname",
line: strings.Join([]string{knownIP.String(), "#" + hostname}, sp),
wantIP: knownIP,
line: ipStr + ` # hostname`,
wantIP: ip,
wantHosts: nil,
}, {
name: "commented_aliases",
line: strings.Join([]string{knownIP.String(), hostname, "#" + alias}, sp),
wantIP: knownIP,
wantHosts: []string{"localhost"},
line: ipStr + ` hostname # alias`,
wantIP: ip,
wantHosts: []string{"hostname"},
}, {
name: "whole_comment",
line: strings.Join([]string{"#", knownIP.String(), hostname}, sp),
line: `# ` + ipStr + ` hostname`,
wantIP: nil,
wantHosts: nil,
}, {
name: "partial_comment",
line: strings.Join([]string{knownIP.String(), hostname[:4] + "#" + hostname[4:]}, sp),
wantIP: knownIP,
wantHosts: []string{hostname[:4]},
line: ipStr + ` host#name`,
wantIP: ip,
wantHosts: []string{"host"},
}, {
name: "empty",
line: ``,
Expand All @@ -579,8 +527,8 @@ func TestUniqueRules_ParseLine(t *testing.T) {
for _, tc := range testCases {
hp := hostsParser{}
t.Run(tc.name, func(t *testing.T) {
ip, hosts := hp.parseLine(tc.line)
assert.True(t, tc.wantIP.Equal(ip))
got, hosts := hp.parseLine(tc.line)
assert.True(t, tc.wantIP.Equal(got))
assert.Equal(t, tc.wantHosts, hosts)
})
}
Expand Down

0 comments on commit 9f341eb

Please sign in to comment.