DataRace detected with concurrent use of client

[ldelossa]

I'm using v0.9.2 of the client. I just encountered the following data race:

==================
WARNING: DATA RACE
Write at 0x00c000350e10 by goroutine 80:
  github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api.(*httpClient).Do.func2()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/client.go:116 +0xfa

Previous write at 0x00c000350e10 by goroutine 83:
  github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api.(*httpClient).Do()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/client.go:122 +0x3e2
  github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1.apiClient.Do()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1/api.go:477 +0x80
  github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1.(*apiClient).Do()
      <autogenerated>:1 +0xb2
  github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1.(*httpAPI).Query()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1/api.go:335 +0x324
  github.com/Paperspace/metrics/prometheus.(*queryStreamer).query()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/prometheus/querystreamer.go:114 +0x15b

Goroutine 80 (running) created at:
  github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api.(*httpClient).Do()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/client.go:115 +0x22a
  github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1.apiClient.Do()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1/api.go:477 +0x80
  github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1.(*apiClient).Do()
      <autogenerated>:1 +0xb2
  github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1.(*httpAPI).Query()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/vendor/github.com/prometheus/client_golang/api/prometheus/v1/api.go:335 +0x324
  github.com/Paperspace/metrics/prometheus.(*queryStreamer).query()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/prometheus/querystreamer.go:114 +0x15b

Goroutine 83 (running) created at:
  github.com/Paperspace/metrics/prometheus.(*queryStreamer).Query()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/prometheus/querystreamer.go:94 +0x1db
  github.com/Paperspace/metrics.(*Poller).Poll()
      /Users/louis/git/go/src/github.com/Paperspace/metrics/poller.go:42 +0x1dd
==================

A little puzzled by this since it looks like the standard golang http client is go routine safe.

Here is the relevant line in my code, tho it is pretty straight forward:

	// issue query
	value, err := q.client.Query(ctx, string(query), time.Now())
	if err != nil {
		log.Printf("session id %s: failed to query prometheus. ERROR: %v QUERY: %v", q.id, err, q)
		return
	}

I will have many go routines call this block. Tho I assumed this was safe.

[ldelossa]

https://github.com/prometheus/client_golang/blob/master/api/client.go#L121

I believe you need to place <-done one level above:

	select {
	case <-ctx.Done():
                <-done
		err = resp.Body.Close()
		if err == nil {
			err = ctx.Err()
		}
	case <-done:
}

Think this bug happened because I pass in a http request scoped context, the client disconnectss, ctx.Done() can be received from so you enter that block and you attempt to Close() the body when the above go routine on line 115 is still reading the resp.Body.

Related PR: #532

[beorn7]

@krasi-georgiev Could you have a look? Thanks 1M.

[krasi-georgiev]

@ldelossa #532 looks valid. Does it fix the race?

[ldelossa]

Little hard to say since my code makes it a tight race but I haven’t seen it since so that’s good.

…

On Mon, Jan 28, 2019 at 3:53 AM Krasi Georgiev ***@***.***> wrote: Closed #531 <#531> via #532 <#532>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#531 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFYalv3JGY5Hl7rPO8Av2Xp8aqxpdvkIks5vHrp_gaJpZM4aUnzY> .

[krasi-georgiev]

ok please reopen if it happens again and will revisit. Please include a full run-able client code preferably with steps to reproduce the race.