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

Data race in ProbeHTTP #922

Closed
ozon2 opened this issue Jun 6, 2022 · 2 comments · Fixed by #927
Closed

Data race in ProbeHTTP #922

ozon2 opened this issue Jun 6, 2022 · 2 comments · Fixed by #927
Assignees
Labels

Comments

@ozon2
Copy link

ozon2 commented Jun 6, 2022

Hi, with blackbox_exporter 0.21.0 I get data races in ProbeHTTP, caused by the new caser used here https://github.com/prometheus/blackbox_exporter/blob/master/prober/http.go#L355.

==================
WARNING: DATA RACE
Write at 0x00c000468c80 by goroutine 28:
  golang.org/x/text/cases.(*titleCaser).Transform()
      /home/tpillot/dev/pkg/mod/golang.org/x/[email protected]/cases/map.go:336 +0x144
  golang.org/x/text/transform.String()
      /home/tpillot/dev/pkg/mod/golang.org/x/[email protected]/transform/transform.go:601 +0x1f8
  golang.org/x/text/cases.Caser.String()
      /home/tpillot/dev/pkg/mod/golang.org/x/[email protected]/cases/cases.go:51 +0x8475
  github.com/prometheus/blackbox_exporter/prober.ProbeHTTP()
      /home/tpillot/dev/pkg/mod/github.com/prometheus/[email protected]/prober/http.go:355 +0x842b

Previous write at 0x00c000468c80 by goroutine 29:
  golang.org/x/text/cases.(*titleCaser).Transform()
      /home/tpillot/dev/pkg/mod/golang.org/x/[email protected]/cases/map.go:336 +0x144
  golang.org/x/text/transform.String()
      /home/tpillot/dev/pkg/mod/golang.org/x/[email protected]/transform/transform.go:601 +0x1f8
  golang.org/x/text/cases.Caser.String()
      /home/tpillot/dev/pkg/mod/golang.org/x/[email protected]/cases/cases.go:51 +0x3075
  github.com/prometheus/blackbox_exporter/prober.ProbeHTTP()
      /home/tpillot/dev/pkg/mod/github.com/prometheus/[email protected]/prober/http.go:428 +0x302b

Here is an example to reproduce the issue:

import (
	"sync"

	"github.com/go-kit/log"
	"github.com/prometheus/blackbox_exporter/config"
        "github.com/prometheus/blackbox_exporter/prober"
)

func TestBlackBox(t *testing.T) {
	logger := log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr))
	mod := config.DefaultModule
	mod.HTTP.Headers = map[string]string{"a": "b"}

	var wg sync.WaitGroup

	probe := func() {
		defer wg.Done()

		prober.ProbeHTTP(context.Background(), "example.com", mod, prometheus.NewRegistry(), logger)
	}

	for i := 0; i < 2; i++ {
		wg.Add(1)

		go probe()
	}

	wg.Wait()
}
@mem
Copy link
Contributor

mem commented Jun 13, 2022

I just ran into this.

It's because the caser is reused. The docs say not to do that.

I'll upload a fix.

mem added a commit that referenced this issue Jun 13, 2022
The cases.Caser returned by calling cases.Title *cannot* be shared among
goroutines. This might happen when Prometheus tries to scrape multiple
targets at the same time. From the docs:

A Caser may be stateful and should therefore not be shared between
goroutines.

Fixes: #922

Signed-off-by: Marcelo E. Magallon <[email protected]>
@mem mem added the bug label Jun 13, 2022
@mem mem self-assigned this Jun 13, 2022
mem added a commit that referenced this issue Jun 13, 2022
The cases.Caser returned by calling cases.Title *cannot* be shared among
goroutines. This might happen when Prometheus tries to scrape multiple
targets at the same time. From the docs:

A Caser may be stateful and should therefore not be shared between
goroutines.

Fixes: #922

Signed-off-by: Marcelo E. Magallon <[email protected]>
@mem
Copy link
Contributor

mem commented Jun 13, 2022

For the record, my own build of BBE doesn't use -race, but this manifests itself as a panic, because one caser overrides the indices of the other and it ends up e.g. with an out of bounds slice access.

mem added a commit to grafana/blackbox_exporter that referenced this issue Jun 14, 2022
The cases.Caser returned by calling cases.Title *cannot* be shared among
goroutines. This might happen when Prometheus tries to scrape multiple
targets at the same time. From the docs:

A Caser may be stateful and should therefore not be shared between
goroutines.

Fixes: prometheus#922

Signed-off-by: Marcelo E. Magallon <[email protected]>
roidelapluie pushed a commit to roidelapluie/blackbox_exporter that referenced this issue Jun 17, 2022
The cases.Caser returned by calling cases.Title *cannot* be shared among
goroutines. This might happen when Prometheus tries to scrape multiple
targets at the same time. From the docs:

A Caser may be stateful and should therefore not be shared between
goroutines.

Fixes: prometheus#922

Signed-off-by: Marcelo E. Magallon <[email protected]>
roidelapluie pushed a commit that referenced this issue Jun 17, 2022
The cases.Caser returned by calling cases.Title *cannot* be shared among
goroutines. This might happen when Prometheus tries to scrape multiple
targets at the same time. From the docs:

A Caser may be stateful and should therefore not be shared between
goroutines.

Fixes: #922

Signed-off-by: Marcelo E. Magallon <[email protected]>
roidelapluie pushed a commit that referenced this issue Jun 17, 2022
The cases.Caser returned by calling cases.Title *cannot* be shared among
goroutines. This might happen when Prometheus tries to scrape multiple
targets at the same time. From the docs:

A Caser may be stateful and should therefore not be shared between
goroutines.

Fixes: #922

Signed-off-by: Marcelo E. Magallon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants