Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix data race in params.Entries chan #5

Merged
merged 2 commits into from
Aug 23, 2019

Conversation

kpp
Copy link

@kpp kpp commented Aug 23, 2019

A solution from #4 (comment)

Other solutions can be:

  1. Create new ServiceEntry in func ensureName if inp from inprogress is already sent
  2. Remove entries with v==inp from map inprogress before sending to channel

Before:

$ go test -race -test.v
=== RUN   TestServer_StartStop
--- PASS: TestServer_StartStop (0.00s)
=== RUN   TestServer_Lookup
==================
WARNING: DATA RACE
Write at 0x00c0001de050 by goroutine 9:
  _/home/humbug/ipfs/mdns.(*client).query()
      /home/humbug/ipfs/mdns/client.go:261 +0x1802
  _/home/humbug/ipfs/mdns.Query()
      /home/humbug/ipfs/mdns/client.go:85 +0xf4
  _/home/humbug/ipfs/mdns.TestServer_Lookup()
      /home/humbug/ipfs/mdns/server_test.go:51 +0x2c2
  testing.tRunner()
      /usr/lib/go-1.12/src/testing/testing.go:865 +0x163

Previous read at 0x00c0001de050 by goroutine 12:
  _/home/humbug/ipfs/mdns.TestServer_Lookup.func1()
      /home/humbug/ipfs/mdns/server_test.go:32 +0x223

Goroutine 9 (running) created at:
  testing.(*T).Run()
      /usr/lib/go-1.12/src/testing/testing.go:916 +0x65a
  testing.runTests.func1()
      /usr/lib/go-1.12/src/testing/testing.go:1157 +0xa8
  testing.tRunner()
      /usr/lib/go-1.12/src/testing/testing.go:865 +0x163
  testing.runTests()
      /usr/lib/go-1.12/src/testing/testing.go:1155 +0x523
  testing.(*M).Run()
      /usr/lib/go-1.12/src/testing/testing.go:1072 +0x2eb
  main.main()
      _testmain.go:64 +0x222

Goroutine 12 (finished) created at:
  _/home/humbug/ipfs/mdns.TestServer_Lookup()
      /home/humbug/ipfs/mdns/server_test.go:26 +0x21f
  testing.tRunner()
      /usr/lib/go-1.12/src/testing/testing.go:865 +0x163
==================
==================
WARNING: DATA RACE
Write at 0x00c0001de058 by goroutine 9:
  _/home/humbug/ipfs/mdns.(*client).query()
      /home/humbug/ipfs/mdns/client.go:266 +0xc02
  _/home/humbug/ipfs/mdns.Query()
      /home/humbug/ipfs/mdns/client.go:85 +0xf4
  _/home/humbug/ipfs/mdns.TestServer_Lookup()
      /home/humbug/ipfs/mdns/server_test.go:51 +0x2c2
  testing.tRunner()
      /usr/lib/go-1.12/src/testing/testing.go:865 +0x163

Previous read at 0x00c0001de058 by goroutine 12:
  _/home/humbug/ipfs/mdns.TestServer_Lookup.func1()
      /home/humbug/ipfs/mdns/server_test.go:35 +0x245

Goroutine 9 (running) created at:
  testing.(*T).Run()
      /usr/lib/go-1.12/src/testing/testing.go:916 +0x65a
  testing.runTests.func1()
      /usr/lib/go-1.12/src/testing/testing.go:1157 +0xa8
  testing.tRunner()
      /usr/lib/go-1.12/src/testing/testing.go:865 +0x163
  testing.runTests()
      /usr/lib/go-1.12/src/testing/testing.go:1155 +0x523
  testing.(*M).Run()
      /usr/lib/go-1.12/src/testing/testing.go:1072 +0x2eb
  main.main()
      _testmain.go:64 +0x222

Goroutine 12 (finished) created at:
  _/home/humbug/ipfs/mdns.TestServer_Lookup()
      /home/humbug/ipfs/mdns/server_test.go:26 +0x21f
  testing.tRunner()
      /usr/lib/go-1.12/src/testing/testing.go:865 +0x163
==================
2019/08/23 17:11:43 [INFO] mdns: Closing client {0xc0000a0048 0xc0000a0050 0xc0000a0058 0xc0000a0060 true 0xc000088420 {1 0}}
2019/08/23 17:11:43 [ERR] mdns: Failed to read packet: read udp4 0.0.0.0:59966: use of closed network connection
2019/08/23 17:11:43 [ERR] mdns: Failed to read packet: read udp6 [::]:46881: use of closed network connection
2019/08/23 17:11:43 [ERR] mdns: Failed to read packet: read udp4 0.0.0.0:5353: use of closed network connection
2019/08/23 17:11:43 [ERR] mdns: Failed to read packet: read udp6 [::]:5353: use of closed network connection
==================
WARNING: DATA RACE
Read at 0x00c00009e3c4 by goroutine 9:
  _/home/humbug/ipfs/mdns.TestServer_Lookup()
      /home/humbug/ipfs/mdns/server_test.go:55 +0x33e
  testing.tRunner()
      /usr/lib/go-1.12/src/testing/testing.go:865 +0x163

Previous write at 0x00c00009e3c4 by goroutine 12:
  _/home/humbug/ipfs/mdns.TestServer_Lookup.func1()
      /home/humbug/ipfs/mdns/server_test.go:38 +0x27c

Goroutine 9 (running) created at:
  testing.(*T).Run()
      /usr/lib/go-1.12/src/testing/testing.go:916 +0x65a
  testing.runTests.func1()
      /usr/lib/go-1.12/src/testing/testing.go:1157 +0xa8
  testing.tRunner()
      /usr/lib/go-1.12/src/testing/testing.go:865 +0x163
  testing.runTests()
      /usr/lib/go-1.12/src/testing/testing.go:1155 +0x523
  testing.(*M).Run()
      /usr/lib/go-1.12/src/testing/testing.go:1072 +0x2eb
  main.main()
      _testmain.go:64 +0x222

Goroutine 12 (finished) created at:
  _/home/humbug/ipfs/mdns.TestServer_Lookup()
      /home/humbug/ipfs/mdns/server_test.go:26 +0x21f
  testing.tRunner()
      /usr/lib/go-1.12/src/testing/testing.go:865 +0x163
==================
--- FAIL: TestServer_Lookup (0.05s)
    testing.go:809: race detected during execution of test
=== RUN   TestNewMDNSService_BadParams
--- PASS: TestNewMDNSService_BadParams (0.00s)
=== RUN   TestMDNSService_BadAddr
--- PASS: TestMDNSService_BadAddr (0.00s)
=== RUN   TestMDNSService_ServiceAddr
--- PASS: TestMDNSService_ServiceAddr (0.00s)
=== RUN   TestMDNSService_InstanceAddr_ANY
--- PASS: TestMDNSService_InstanceAddr_ANY (0.00s)
=== RUN   TestMDNSService_InstanceAddr_SRV
--- PASS: TestMDNSService_InstanceAddr_SRV (0.00s)
=== RUN   TestMDNSService_InstanceAddr_A
--- PASS: TestMDNSService_InstanceAddr_A (0.00s)
=== RUN   TestMDNSService_InstanceAddr_AAAA
--- PASS: TestMDNSService_InstanceAddr_AAAA (0.00s)
=== RUN   TestMDNSService_InstanceAddr_TXT
--- PASS: TestMDNSService_InstanceAddr_TXT (0.00s)
=== RUN   TestMDNSService_HostNameQuery
--- PASS: TestMDNSService_HostNameQuery (0.00s)
=== RUN   TestMDNSService_serviceEnum_PTR
--- PASS: TestMDNSService_serviceEnum_PTR (0.00s)
FAIL
exit status 1

After:

$ go test -race -test.v
=== RUN   TestServer_StartStop
--- PASS: TestServer_StartStop (0.00s)
=== RUN   TestServer_Lookup
2019/08/23 17:12:28 [INFO] mdns: Closing client {0xc0000a0048 0xc0000a0050 0xc0000a0058 0xc0000a0060 true 0xc000088420 {1 0}}
2019/08/23 17:12:28 [ERR] mdns: Failed to read packet: read udp4 0.0.0.0:57955: use of closed network connection
2019/08/23 17:12:28 [ERR] mdns: Failed to read packet: read udp4 0.0.0.0:5353: use of closed network connection
2019/08/23 17:12:28 [ERR] mdns: Failed to read packet: read udp6 [::]:43540: use of closed network connection
2019/08/23 17:12:28 [ERR] mdns: Failed to read packet: read udp6 [::]:5353: use of closed network connection
--- PASS: TestServer_Lookup (0.06s)
=== RUN   TestNewMDNSService_BadParams
--- PASS: TestNewMDNSService_BadParams (0.00s)
=== RUN   TestMDNSService_BadAddr
--- PASS: TestMDNSService_BadAddr (0.00s)
=== RUN   TestMDNSService_ServiceAddr
--- PASS: TestMDNSService_ServiceAddr (0.00s)
=== RUN   TestMDNSService_InstanceAddr_ANY
--- PASS: TestMDNSService_InstanceAddr_ANY (0.00s)
=== RUN   TestMDNSService_InstanceAddr_SRV
--- PASS: TestMDNSService_InstanceAddr_SRV (0.00s)
=== RUN   TestMDNSService_InstanceAddr_A
--- PASS: TestMDNSService_InstanceAddr_A (0.00s)
=== RUN   TestMDNSService_InstanceAddr_AAAA
--- PASS: TestMDNSService_InstanceAddr_AAAA (0.00s)
=== RUN   TestMDNSService_InstanceAddr_TXT
--- PASS: TestMDNSService_InstanceAddr_TXT (0.00s)
=== RUN   TestMDNSService_HostNameQuery
--- PASS: TestMDNSService_HostNameQuery (0.00s)
=== RUN   TestMDNSService_serviceEnum_PTR
--- PASS: TestMDNSService_serviceEnum_PTR (0.00s)
PASS

@kpp kpp mentioned this pull request Aug 23, 2019
@kpp kpp changed the title Fix data race in params.Entries chan WIP: Fix data race in params.Entries chan Aug 23, 2019
@kpp kpp changed the title WIP: Fix data race in params.Entries chan Fix data race in params.Entries chan Aug 23, 2019
Copy link
Collaborator

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Stebalien Stebalien merged commit 23958d6 into whyrusleeping:master Aug 23, 2019
@kpp kpp deleted the fix_data_race branch August 24, 2019 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants