panic in Close on windows

[acud]

Aloha again :wave:

We've been seeing a lot of these lately on our windows CI build. It runs without race detector on github actions.

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0x1d247ed]

goroutine 10507 [running]:
testing.tRunner.func1.1(0x1f6aa80, 0x1e38850)
	C:/hostedtoolcache/windows/go/1.15.8/x64/src/testing/testing.go:1072 +0x310
testing.tRunner.func1(0xc000fd1080)
	C:/hostedtoolcache/windows/go/1.15.8/x64/src/testing/testing.go:1075 +0x43a
panic(0x1f6aa80, 0x1e38850)
	C:/hostedtoolcache/windows/go/1.15.8/x64/src/runtime/panic.go:969 +0x1c7
github.com/foxcpp/go-mockdns.(*Server).Close(0x0, 0x0, 0x0)
	C:/Users/runneradmin/go/pkg/mod/github.com/foxcpp/[email protected]/server.go:414 +0x2d
runtime.Goexit()
	C:/hostedtoolcache/windows/go/1.15.8/x64/src/runtime/panic.go:617 +0x1e5
testing.(*common).SkipNow(0xc000fd1080)
	C:/hostedtoolcache/windows/go/1.15.8/x64/src/testing/testing.go:832 +0x45
testing.(*common).Skipf(0xc000fd1080, 0x20fc744, 0x24, 0xc000c0fba8, 0x1, 0x1)
	C:/hostedtoolcache/windows/go/1.15.8/x64/src/testing/testing.go:818 +0x97
github.com/ethersphere/bee/pkg/p2p/libp2p_test.TestStaticAddressResolver(0xc000fd1080)
	D:/a/bee/bee/pkg/p2p/libp2p/static_resolver_test.go:105 +0x525
testing.tRunner(0xc000fd1080, 0x2268a30)
	C:/hostedtoolcache/windows/go/1.15.8/x64/src/testing/testing.go:1123 +0xef
created by testing.(*T).Run
	C:/hostedtoolcache/windows/go/1.15.8/x64/src/testing/testing.go:1168 +0x2b3

[foxcpp]

It seems like in both cases you are calling methods on a nil Server object.

[acud]

In the case of the windows panic the test was constructed like so:

srv, _ := mockdns.NewServer(map[string]mockdns.Zone{
	"ipv4.com.": {
			A: []string{"192.168.1.34"},
	},
	"ipv4and6.com.": {
			A:    []string{"192.168.1.34"},
			AAAA: []string{"2001:db8::8a2e:370:1111"},
	},
}, false)
defer srv.Close()


... check if the test runs in windows and if yes - t.Skip() it

So the call to srv.Close() was made after the server was created but before it was patched.

In the case of the mac flake - this is happening on an already modified version of the test, which is doing the patch and unpatch calls, so I'm really not sure why this is happening. The test can be seen here

[foxcpp]

The obvious possibility is NewServer failing for some reason and returning a nil srv. Errors should always be checked. Why NewServer fails on Windows is the main question here.

[acud]

Nice catch. Added the error check but did not manage to get around to actually checking it, but we just got this on the windows CI, right now it seems to be failing with the following: static_resolver_test.go:105: new mockdns: listen udp4 127.0.0.1:50423: bind: An attempt was made to access a socket in a way forbidden by its access permissions.

[foxcpp]

I'm not familiar with Windows that much, perhaps CI restricts the use of UDP sockets? Looks like binding on TCP port succeeds but UDP fails.