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

runtime: iterator returns map entries after clear (pre-swissmap) #70189

Closed
randall77 opened this issue Nov 4, 2024 · 6 comments
Closed

runtime: iterator returns map entries after clear (pre-swissmap) #70189

randall77 opened this issue Nov 4, 2024 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@randall77
Copy link
Contributor

Go version

tip

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/khr/Library/Caches/go-build'
GOENV='/Users/khr/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/khr/gopath/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/khr/gopath'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/khr/sandbox/ro5'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/khr/sandbox/ro5/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='devel go1.24-fb5dcc1526 Mon Nov 4 12:50:55 2024 -0800'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/khr/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/khr/sandbox/ro5/src/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/z9/dty110711l9cr9w3ktv1_2380000gn/T/go-build1711499111=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

package main

func nan() float64 {
	var x, y float64
	return x / y
}

func main() {
	m := map[float64]int{}

	// Make a small map with nan keys
	for i := 0; i < 8; i++ {
		m[nan()] = i
	}

	// Start iterating on it.
	start := true
	for k, v := range m {
		if start {
			// Add some more elements.
			for i := 0; i < 10; i++ {
				m[float64(i)] = i
			}
			// Now clear the map.
			clear(m)
			start = false
		} else {
			// This should never print anything.
			println(k, v)
		}
	}
}

When run, this should never print anything. The map is cleared during the first iteration. No map elements should survive to cause a second iteration.

There is no bug now that we've switched by default to swiss maps. The old maps, which you can get with GOEXPERIMENT=noswissmap, have this problem. This problem is present in 1.22.6 and 1.23.2. Presumably it has been there since clear was introduced in 1.21.

Seems related to #59411. Somehow the fix for that issue doesn't help here.

@prattmic

What did you see happen?

NaN 3
NaN 4
NaN 5
NaN 6
NaN 7
NaN 0
NaN 1

What did you expect to see?

nothing printed

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 4, 2024
@randall77
Copy link
Contributor Author

Looks like the fix in #59411 is only a partial one. In this case the oldbuckets field is already zeroed (because growth is done) before clear is called. I'm not sure how we'd fix it to make it complete, other than just by copying what swissmap does with a counter incremented on each clear.

@fengyoulin
Copy link
Contributor

I haven't found a better way, and I'm trying to implement it according to your idea.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/625275 mentions this issue: runtime: fix iterator returns map entries after clear (pre-swissmap)

@mknyszek mknyszek moved this to All-But-Submitted in Go Compiler / Runtime Nov 6, 2024
@mknyszek mknyszek added this to the Go1.24 milestone Nov 6, 2024
@mknyszek mknyszek added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 6, 2024
@github-project-automation github-project-automation bot moved this from All-But-Submitted to Done in Go Compiler / Runtime Nov 12, 2024
@jianfengtony
Copy link

As i test, this issue also affect go1.22.10, so why not backport it to this version? thanks.

@ianlancetaylor
Copy link
Member

@jianfengtony Our backport guidelines can be found at https://go.dev/wiki/MinorReleases. Do you think this meets the guidelines for backporting? If so, that wiki page shows how to request a backport. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Development

No branches or pull requests

7 participants