dnscrypt icon indicating copy to clipboard operation
dnscrypt copied to clipboard

Incorrect use of sync.WaitGroup

Open ainar-g opened this issue 10 months ago • 0 comments

Caught in dnsproxy tests failures:

==================
WARNING: DATA RACE
Read at 0x00c0001823e0 by goroutine 9:
  runtime.raceread()
      <autogenerated>:1 +0x24
  github.com/ameshkov/dnscrypt/v2.(*Server).ServeTCP()
      /Users/runner/go/pkg/mod/github.com/ameshkov/dnscrypt/[email protected]/server_tcp.go:65 +0x1ea
  github.com/AdguardTeam/dnsproxy/upstream.startTestDNSCryptServer.func3()
      /Users/runner/work/dnsproxy/dnsproxy/upstream/dnscrypt_test.go:79 +0x45

Previous write at 0x00c0001823e0 by goroutine 10:
  runtime.racewrite()
      <autogenerated>:1 +0x24
  github.com/ameshkov/dnscrypt/v2.(*Server).Shutdown.func1()
      /Users/runner/go/pkg/mod/github.com/ameshkov/dnscrypt/[email protected]/server.go:139 +0x44

Goroutine 9 (running) created at:
  github.com/AdguardTeam/dnsproxy/upstream.startTestDNSCryptServer()
      /Users/runner/work/dnsproxy/dnsproxy/upstream/dnscrypt_test.go:78 +0x91b
  github.com/AdguardTeam/dnsproxy/upstream.TestDNSCrypt_Exchange_dialFail.func2()
      /Users/runner/work/dnsproxy/dnsproxy/upstream/dnscrypt_test.go:192 +0xd5
  testing.tRunner()
      /Users/runner/hostedtoolcache/go/1.20.10/x64/src/testing/testing.go:1576 +0x216
  testing.(*T).Run.func1()
      /Users/runner/hostedtoolcache/go/1.20.10/x64/src/testing/testing.go:1629 +0x47

Goroutine 10 (running) created at:
  github.com/ameshkov/dnscrypt/v2.(*Server).Shutdown()
      /Users/runner/go/pkg/mod/github.com/ameshkov/dnscrypt/[email protected]/server.go:138 +0x14e
  github.com/AdguardTeam/dnsproxy/upstream.startTestDNSCryptServer.func1()
      /Users/runner/work/dnsproxy/dnsproxy/upstream/dnscrypt_test.go:54 +0x9c
  github.com/AdguardTeam/golibs/testutil.CleanupAndRequireSuccess.func1()
      /Users/runner/go/pkg/mod/github.com/!adguard!team/[email protected]/testutil/testutil.go:216 +0x46
  testing.(*common).Cleanup.func1()
      /Users/runner/hostedtoolcache/go/1.20.10/x64/src/testing/testing.go:1150 +0x193
  testing.(*common).runCleanup()
      /Users/runner/hostedtoolcache/go/1.20.10/x64/src/testing/testing.go:1328 +0x1e9
  testing.tRunner.func2()
      /Users/runner/hostedtoolcache/go/1.20.10/x64/src/testing/testing.go:1570 +0x52
  runtime.deferreturn()
      /Users/runner/hostedtoolcache/go/1.20.10/x64/src/runtime/panic.go:476 +0x32
  testing.(*T).Run.func1()
      /Users/runner/hostedtoolcache/go/1.20.10/x64/src/testing/testing.go:1629 +0x47
==================

Which results in:

panic: sync: WaitGroup is reused before previous Wait has returned

I think that this is because s.wg.Wait() is called in a goroutine, and there is no guarantee that it will be run after all ServeTCP calls.

ainar-g avatar Nov 01 '23 15:11 ainar-g