time: datarace on `localLoc` between `initLocal` and a library following private fields through `reflect`

[dhiaayachi]

What version of Go are you using (go version)?

$ go version
go version go1.24.4 darwin/arm64

Does this issue reproduce with the latest release?

yes, go 1.24.4 is latest.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users//Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/myuser/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/j7/py1yk76x32sb49vv6cdltx2m0000gn/T/go-build1644786394=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/myuser/git/test-race/go.mod'
GOMODCACHE='/Users/myuser/.gvm/pkgsets/go1.24.4/global/pkg/mod'
GONOPROXY='github.com/hashicorp/*,github.com/dhiaayachi/*'
GONOSUMDB='github.com/hashicorp/*,github.com/dhiaayachi/*'
GOOS='darwin'
GOPATH='/Users/myuser/.gvm/pkgsets/go1.24.4/global'
GOPRIVATE='github.com/hashicorp/*,github.com/dhiaayachi/*'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/myuser/.gvm/gos/go1.24.4'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/myuser/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/myuser/.gvm/gos/go1.24.4/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.4'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Running this program with the race detector activated (go run -race main.go) result on a data race being detected. I tried to remove the dependency to go-cmp but I was unsuccessfully able to reproduce it without it, that said the stack trace show that it can be reproduced without go-cmp given the right use of the reflect package.

Adding

	t1 := time.Now()
	fmt.Println(t1)

before starting the 2 go routine resolve the race condition.

What did you expect to see?

No data race detected

What did you see instead?

A data race detected with the following output:

the details of the data race are the following:

race Output

WARNING: DATA RACE

Write at 0x000102ecd2c8 by goroutine 8:

time.initLocal()

/Users/myuser/.gvm/gos/go1.23.7/src/time/zoneinfo_unix.go:41 +0x394

sync.(*Once).doSlow()

/Users/myuser/.gvm/gos/go1.23.7/src/sync/once.go:76 +0xac

sync.(*Once).Do()

/Users/myuser/.gvm/gos/go1.23.7/src/sync/once.go:67 +0x40

time.(*Location).get()

/Users/myuser/.gvm/gos/go1.23.7/src/time/zoneinfo.go:96 +0x70

time.Time.locabs()

/Users/myuser/.gvm/gos/go1.23.7/src/time/time.go:494 +0x74

time.Time.appendFormat()

/Users/myuser/.gvm/gos/go1.23.7/src/time/format.go:668 +0x58

time.Time.AppendFormat()

/Users/myuser/.gvm/gos/go1.23.7/src/time/format.go:662 +0xf8

time.Time.Format()

/Users/myuser/.gvm/gos/go1.23.7/src/time/format.go:648 +0xa4

time.Time.String()

/Users/myuser/.gvm/gos/go1.23.7/src/time/format.go:546 +0x4c

time.(*Time).String()

:1 +0x4c

github.com/google/go-cmp/cmp.formatOptions.FormatValue.func2()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_reflect.go:144 +0x110

github.com/google/go-cmp/cmp.formatOptions.FormatValue()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_reflect.go:147 +0x31c

github.com/google/go-cmp/cmp.formatOptions.FormatDiff()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_compare.go:125 +0x598

github.com/google/go-cmp/cmp.formatOptions.formatDiffList()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_compare.go:315 +0x2350

github.com/google/go-cmp/cmp.formatOptions.FormatDiff()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_compare.go:177 +0x159c

github.com/google/go-cmp/cmp.formatOptions.FormatDiff()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_compare.go:192 +0x1770

github.com/google/go-cmp/cmp.(*defaultReporter).String()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report.go:45 +0xf0

github.com/google/go-cmp/cmp.Diff()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/compare.go:132 +0x20c

github.com/dhiaayachi/test-race.TestRace.func1()

/Users/myuser/git/test-race/main_test.go:47 +0x17c

github.com/dhiaayachi/test-race.TestRace.func4()

/Users/myuser/git/test-race/main_test.go:78 +0x44
Previous read at 0x000102ecd2c8 by goroutine 7:

reflect.Value.lenNonSlice()

/Users/myuser/.gvm/gos/go1.23.7/src/reflect/value.go:1775 +0x118

reflect.Value.Len()

/Users/myuser/.gvm/gos/go1.23.7/src/reflect/value.go:1761 +0x580

reflect.Value.IsZero()

/Users/myuser/.gvm/gos/go1.23.7/src/reflect/value.go:1614 +0x588

github.com/google/go-cmp/cmp.formatOptions.FormatValue()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_reflect.go:194 +0x2220

github.com/google/go-cmp/cmp.formatOptions.FormatValue()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_reflect.go:298 +0x1804

github.com/google/go-cmp/cmp.formatOptions.FormatValue()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_reflect.go:205 +0x240c

github.com/google/go-cmp/cmp.formatOptions.FormatValue()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_reflect.go:205 +0x240c

github.com/google/go-cmp/cmp.formatOptions.FormatValue()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_reflect.go:298 +0x1804

github.com/google/go-cmp/cmp.formatOptions.FormatValue()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_reflect.go:308 +0x1180

github.com/google/go-cmp/cmp.formatOptions.FormatDiff()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report_compare.go:144 +0x2068

github.com/google/go-cmp/cmp.(*defaultReporter).String()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/report.go:45 +0xf0

github.com/google/go-cmp/cmp.Diff()

/Users/myuser/.gvm/pkgsets/go1.23.7/global/pkg/mod/github.com/google/go-cmp@v0.7.0/cmp/compare.go:132 +0x20c

github.com/dhiaayachi/test-race.TestRace.func2()

/Users/myuser/git/test-race/main_test.go:56 +0xe8

github.com/dhiaayachi/test-race.TestRace.func3()

/Users/myuser/git/test-race/main_test.go:65 +0x44
Goroutine 8 (running) created at:

github.com/dhiaayachi/test-race.TestRace()

/Users/myuser/git/test-race/main_test.go:72 +0x17c

testing.tRunner()

/Users/myuser/.gvm/gos/go1.23.7/src/testing/testing.go:1690 +0x184

testing.(*T).Run.gowrap1()

/Users/myuser/.gvm/gos/go1.23.7/src/testing/testing.go:1743 +0x40
Goroutine 7 (running) created at:

github.com/dhiaayachi/test-race.TestRace()

/Users/myuser/git/test-race/main_test.go:58 +0xdc

testing.tRunner()

/Users/myuser/.gvm/gos/go1.23.7/src/testing/testing.go:1690 +0x184

testing.(*T).Run.gowrap1()

/Users/myuser/.gvm/gos/go1.23.7/src/testing/testing.go:1743 +0x40

[Jorropo]

Am I missing where I can find this program ?

[dhiaayachi]

Sorry missed adding the link, updated!

[gabyhelp]

Related Issues

• time: race in Now #4622 (closed)
• reflect: spurious looking race condition #15832 (closed)
• Concurrent calls to errors.As results in a data race #38293 (closed)
• runtime: race detector mis-identifies `runtime.racereadrange()` as the read location #51459 (closed)
• testing: data race between parallel panicking and normal subtest #37551 (closed)
• runtime/metrics: data race detected in Read #53542 (closed)
• Data race in `singleflight.Group` when checking call duplicates #49798 (closed)
• cmd/compile/internal: DATA RACE if `-coverprofile` flag is provided #56370 (closed)
• reflect: Method() on a type races #56011 (closed)
• html/template: Execute is not concurrent safe #51344 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[Jorropo]

Your library is accessing time.loc.* there are two levels of accessing private fields (this matters because you can't have this data race if you use == between time.Time types since time.loc is a pointer altho I don't know if it timezone comparisons would return accurate results then).

Our usual stance on issues relating to accessing private fields of stdlib structs is fixing this would set a bad precedent because it would severely limit our ability to do any kind of modification to the internals of the stdlib.

I am gonna close this but if someone thinks this can be solved and this is something we want to do it could be reopened.

[dhiaayachi]

Thank you for the explanation @Jorropo!

My understanding is that even if time.Time contain a pointer, it's considered safe to shallow copy as it's immutable and by extension it's also correct to perform an == on it, right?

So based on that, if I understand right, your suggestion is to "fix" go-cmp to treat time.Time as a special case and avoid walking through its fields and use == when comparing time.Time?

btw, I don't have any affiliation with go-cmp, I'm just a user of the library.

[randall77]

I kind of feel like this is a problem. Not sure how to fix it though. The problem is we have paths that get around this sync.Once.

Here's a simplified example:

package main

import (
	"reflect"
	"time"
)

func main() {
	c := make(chan bool)
	go func() {
		println(time.Local.String())
		c <- true
	}()
	go func() {
		println(reflect.DeepEqual(time.Local, time.UTC))
		c <- true
	}()
	<-c
	<-c
}

The first goroutine goes through the sync.Once to initialize the local time zone. The latter goes through reflect so it doesn't know that there is a sync.Once guarding access to the time zone info.

I'm going to reopen. But I'm not sure how we might go about fixing this. Or even if we should. Jorropo's warning about accessing internal fields is pertinent here.

Maybe we need to move some of this sync.Once work into an init function? That would help, I think, but maybe there are reasons we don't want to do this processing on startup for ~every binary in existence.

[randall77]

(As a practical matter, you can add time.Local.String(), or probably any other time function, at the start of main.main and this problem will go away.)

[ianlancetaylor]

In general it's not reliable to use reflect.DeepEqual with types that you don't control, because it compares unexported struct fields. That means that the result of reflect.DeepEqual can change if the package changes the ways that it uses unexported fields.

And, of course, reflect.DeepEqual does not work for time.Time values, for the reasons given at https://pkg.go.dev/time#Time in the discussion of the == operator.

I don't think there is anything to do here.

[seankhliao]

this looks like another instance of #71732

[Jorropo]

It's a bit different to #71732 since this code never calls reflect.DeepEqual.