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

affected/package: sync #50544

Closed
pengpeng1993 opened this issue Jan 11, 2022 · 4 comments
Closed

affected/package: sync #50544

pengpeng1993 opened this issue Jan 11, 2022 · 4 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@pengpeng1993
Copy link

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

$ go version
go version go1.17.5 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/Users/liulu/Documents/learn/bin"
GOCACHE="/Users/liulu/Library/Caches/go-build"
GOENV="/Users/liulu/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/liulu/Documents/learn/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/didi/Documents/learn"
GOPRIVATE=""
GOPROXY="https://goproxy.cn,https://mirrors.aliyun.com/goproxy/"
GOROOT="/usr/local/go"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.5"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/jz/gm2fxw8x4dqg1lg9ydyxl4w40000gn/T/go-build2365601841=/tmp/go-build -gno-record-gcc-switches -fno-commo

What did you do?

What did you expect to see?

When I learned the add function, I see the code of panic as follows:

if w != 0 && delta > 0 && v == int32(delta) {
    panic("sync: WaitGroup misuse: Add called concurrently with Wait")
}
w, the number of waiter, is the number of waiting goroutines. There are only two operations: adding 1 and setting zero, so it must be greater than or equal to 0. The first time you use add function, such as add (n), you can see v = n, w = 0, and w != 0 indicates that the function of wait has been executed, that is, it is not the first addition operation;

delta > 0 indicates that this is an addition operation. If v == int32(delta), that is, v + delta == delta, deduces v = 0, it may be the first time to add () or execute add (- 1) to reduce v to 0.

So I think the condition for this painc trigger should be two add concurrency. The first add reduces couter to 0, when the second add adds 1, the second will trigger painc here.

What did you see instead?

The code comment of panic should be changed as

if w != 0 && delta > 0 && v == int32(delta) {
    panic("WaitGroup misuse: Multiple add called concurrently")
}

Additional

I am new to use golang, and not sure if I understand this correctly, so let me know if i am wrong.

thanks a lot!

@davecheney
Copy link
Contributor

Can you please provide a runnable code sample that demonstrates the problem. Thank you.

@davecheney davecheney added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 11, 2022
@pengpeng1993
Copy link
Author

Can you please provide a runnable code sample that demonstrates the problem. Thank you.
assume w =1 ,couter = 1 two add concurrently
first add(1)
second add (-1) will painc
when first add reduces couter to 0 then first add suspend ,other add coming atomic Add successful will trigger painc. suppose other add's parameter is positive number.

func (wg *WaitGroup) Add(delta int) {
	statep, semap := wg.state()
	if race.Enabled {
		_ = *statep // trigger nil deref early
		if delta < 0 {
			// Synchronize decrements with Wait.
			race.ReleaseMerge(unsafe.Pointer(wg))
		}
		race.Disable()
		defer race.Enable()
	}
	state := atomic.AddUint64(statep, uint64(delta)<<32)
        // (1) the first one suspend there, but couter has been change .other one call Add(1) assume positive number
	v := int32(state >> 32)
	w := uint32(state)
	if race.Enabled && delta > 0 && v == int32(delta) {
		// The first increment must be synchronized with Wait.
		// Need to model this as a read, because there can be
		// several concurrent wg.counter transitions from 0.
		race.Read(unsafe.Pointer(semap))
	}
	if v < 0 {
		panic("sync: negative WaitGroup counter")
	}
       // (2) other one will  succeed atomic Add ,then painc on this 
      // w > 0 delta > 0  && v=1 delta=1
	if w != 0 && delta > 0 && v == int32(delta) {
		panic("sync: WaitGroup misuse: Add called concurrently with Wait")
	}
	if v > 0 || w == 0 {
		return
	}
	// This goroutine has set counter to 0 when waiters > 0.
	// Now there can't be concurrent mutations of state:
	// - Adds must not happen concurrently with Wait,
	// - Wait does not increment waiters if it sees counter == 0.
	// Still do a cheap sanity check to detect WaitGroup misuse.
	if *statep != state {
		panic("sync: WaitGroup misuse: Add called concurrently with Wait")
	}
	// Reset waiters count to 0.
	*statep = 0
	for ; w != 0; w-- {
		runtime_Semrelease(semap, false, 0)
	}
}

but in this case ,Add called concurrently with Add ?
I am new to use golang, and not sure if I understand this correctly, so let me know if i am wrong. thanks

@davecheney
Copy link
Contributor

davecheney commented Jan 11, 2022

Thank you, sadly this is not a runnable code sample. If I understand your bug report correctly, then you should be able to write e short program that demonstrates the scenario you are describing

@davecheney davecheney changed the title affected/package: affected/package: sync Jan 11, 2022
@seankhliao
Copy link
Member

Duplicate of #50525

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals only.

For questions please refer to https://github.com/golang/go/wiki/Questions

@golang golang locked and limited conversation to collaborators Jan 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants