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

testing: calling TempDir() from a Benchmark causes it to fail #41062

Closed
hannahhoward opened this issue Aug 27, 2020 · 6 comments
Closed

testing: calling TempDir() from a Benchmark causes it to fail #41062

hannahhoward opened this issue Aug 27, 2020 · 6 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@hannahhoward
Copy link

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

$ go version
go version go1.15 darwin/amd64

Does this issue reproduce with the latest release?

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

go env Output
$ go env

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/hannah/Library/Caches/go-build"
GOENV="/Users/hannah/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/hannah/projects/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/hannah/projects/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.15/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.15/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/hannah/projects/gosrc/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/qj/9f3_14352b1b07rxs4pm9tmc0000gn/T/go-build263434137=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

  • wrote a benchmark that uses B.TempDir

What did you expect to see?

  • success, assuming no other failures

What did you see instead?

  • testing.go:905: TempDir: mkdir /var/folders/qj/9f3_14352b1b07rxs4pm9tmc0000gn/T/BenchmarkRoundtripSuccess_test-20-10000723322760/003: no such file or directory

I dug into the benchmark, determined the first run succeeded, but on a short benchmark that is run again with N>1, the failure happens.

I dug into the benchmark.go source code and I believe the problem is that after every benchmark is run, common.runCleanup is called. runCleanup will delete the top level source folder under which TempDir() creates more temp directories. However, the top level source directory is only created once due to the sync.Once mutex.

I went ahead and wrote a test and possible fix which I submitted to Gerril -- https://go-review.googlesource.com/c/go/+/250950

I have no idea if the fix is right (it does make the test pass) but the test should demonstrate the problem.

@bcmills bcmills changed the title Calling TempDir() from Benchmark cause benchmark to fail testing: calling TempDir() from a Benchmark causes it to fail Aug 27, 2020
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 27, 2020
@bcmills bcmills added this to the Go1.16 milestone Aug 27, 2020
@bcmills
Copy link
Contributor

bcmills commented Aug 27, 2020

Neat find! This also applies to calls to (*testing.T).TempDir within a Cleanup callback: https://play.golang.org/p/_qWZd56AwyP

CC @ianlancetaylor @rogpeppe @bradfitz

@hannahhoward
Copy link
Author

hannahhoward commented Aug 27, 2020

For sure! I submitted the PR on Gerrill as a demo of the issue and a possible fix. Unfortunately, I discovered it while writing benchmarks for my job that I definitely need to get back to. So I probably won't have time to go through the revision process for the PR -- but I suspect you all will find a more ideal solution :) Please let me know if you need any more from me.

In the mean time, I'll just workaround with ioutil.

@bcmills
Copy link
Contributor

bcmills commented Aug 27, 2020

Thanks for at least getting us started with a report and a solid test case!

@gopherbot
Copy link

Change https://golang.org/cl/251297 mentions this issue: testing: fix TempDir failure in Cleanup and Benchmark

@anacrolix
Copy link
Contributor

Any chance this would be backported to Go 1.15?

@ianlancetaylor
Copy link
Contributor

We usually only backport changes that are serious bugs that don't have a workaround, and this one has a pretty easy workaround.

@golang golang locked and limited conversation to collaborators Jan 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants