runtime/race: TestOutput failures involving missing output for external_cgo_thread

[bcmills]

##### Testing race detector
--- FAIL: TestOutput (6.12s)
    output_test.go:76: failed test case external_cgo_thread, expect:
        ==================
        WARNING: DATA RACE
        Read at 0x[0-9,a-f]+ by main goroutine:
          main\.main\(\)
              .*/main\.go:34 \+0x[0-9,a-f]+
        
        Previous write at 0x[0-9,a-f]+ by goroutine [0-9]:
          main\.goCallback\(\)
              .*/main\.go:27 \+0x[0-9,a-f]+
          _cgoexp_[0-9a-z]+_goCallback\(\)
              .*_cgo_gotypes\.go:[0-9]+ \+0x[0-9,a-f]+
        
        Goroutine [0-9] \(running\) created at:
          runtime\.newextram\(\)
              .*/runtime/proc.go:[0-9]+ \+0x[0-9,a-f]+
        ==================
        got:
FAIL
FAIL	runtime/race	6.171s

2020-12-04T08:49:16-b67b7dd/linux-arm64-aws
2020-04-30T18:12:03-2491c5f/linux-arm64-packet
2020-01-06T16:23:27-e8729a7/linux-amd64-fedora
2019-10-23T07:43:18-ab3f1a2/linux-amd64
2019-05-15T19:53:15-3e2c522/linux-ppc64le-buildlet
2019-05-15T17:02:39-ba23fa4/linux-ppc64le-buildlet

CC @dvyukov @dfava @aclements

[dfava]

Looking at external_cgo_thread in output_test.go, there is quite a bit that I don't understand. But I can say that it doesn't look related to the change in CL 220419 (https://go-review.googlesource.com/c/go/+/220419) or the regression it caused (discussion in #42598) and later fixed in 271987 (https://go-review.googlesource.com/c/go/+/271987).

The reason being that CL 220419 and 271987 modify the calls to the data race detector when channels are used to synchronize. Channels are not involved in external_cgo_thread, so the test does not exercise the code paths modified in 220419 and 271987.

I would think that whether the increment to racy++ happens first in the callBack or in main doesn't matter. As far as I understand, the race detector should flag the race regardless. Following this logic, the time.Seep() in main could be placed after the increment to racy++.

The sleep is making sure main does not finish before the callBack() has a chance to run. Is there a way to block until callBack() has run? Blocking would be more bullet-proof than sleeping. We can even add a synchronization here to make sure callBack finishes. I haven't tested this, but something like:

...
var done chan bool
var racy int

//export goCallback
func goCallback() {
  racy++
  done <- true
}

func main() {
  done = make(chan bool)
  var c C.cb
  C.startThread(&c)
  racy++
  <- done
}

@dvyukov , does such a change to the test sound reasonable?

[gopherbot]

Change https://golang.org/cl/276752 mentions this issue: test: make a race detector test robust to timing variations