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: NumCgoCall only counts calls from threads that are still alive #46789

Closed
rhysh opened this issue Jun 16, 2021 · 2 comments
Closed

runtime: NumCgoCall only counts calls from threads that are still alive #46789

rhysh opened this issue Jun 16, 2021 · 2 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rhysh
Copy link
Contributor

rhysh commented Jun 16, 2021

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

$ go1.16 version
go version go1.16.5 darwin/amd64
$ go-tip version
go version devel go1.17-a294e4e798 Wed Jun 16 16:38:43 2021 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes, this is present in the most recent stable release (go1.16.5) and in the most recent development commit (go1.17-a294e4e798, after go1.17beta1).

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

go env Output
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/rhys/Library/Caches/go-build"
GOENV="/Users/rhys/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/rhys/go/pkg/mod"
GONOPROXY="*"
GONOSUMDB="*"
GOOS="darwin"
GOPATH="/Users/rhys/go"
GOPRIVATE="*"
GOPROXY="direct"
GOROOT="/Users/rhys/go/version/go1.16"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/Users/rhys/go/version/go1.16/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16.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/49/zmds5zsn75z1283vtzxyfr5hj7yjq4/T/go-build2625819705=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I forced the runtime to create a thread, did a cgo call on that thread, and then terminated the thread. I asked the runtime for a count of "the number of cgo calls made by the current process" via runtime.NumCgoCall before making the cgo call, after making the cgo call but before triggering the runtime to destroy the thread, and again after I expected the the thread to have exited.

What did you expect to see?

I expected the result of runtime.NumCgoCall to increase by 1 for each cgo call in my program, and to never decrease.

What did you see instead?

The result of runtime.NumCgoCall only reflects calls made by threads that are still alive, so decreases when threads that have made cgo calls exit.


$ go1.16 run .
our count of calls 0 100 100
runtime.NumCgoCall 1 101 1

$ go-tip run .
our count of calls 0 100 100
runtime.NumCgoCall 1 101 1
package main

/*
void call() {}
*/
import "C"

import (
	"fmt"
	"runtime"
	"sync"
	"sync/atomic"
	"time"
)

func init() {
	// Excuse the process's main thread from executing the test code
	runtime.LockOSThread()
}

func main() {
	var ourCount int64
	step1 := func() {
		// Force to run on a new thread
		runtime.LockOSThread()
		C.call()
		atomic.AddInt64(&ourCount, 1)
	}
	step2 := func() {
		// Trigger thread to mexit, due to unbalanced {Lock,Unlock}OSThread calls
		runtime.Goexit()
	}

	var (
		begin   = make(chan struct{})
		first   sync.WaitGroup
		proceed = make(chan struct{})
		final   sync.WaitGroup
	)
	for i := 0; i < 100; i++ {
		first.Add(1)
		final.Add(1)
		go func() {
			defer final.Done()
			<-begin
			step1()
			first.Done()
			<-proceed
			step2()
		}()
	}

	cgoStart := runtime.NumCgoCall()
	ourStart := atomic.LoadInt64(&ourCount)
	close(begin)
	first.Wait()
	cgoMiddle := runtime.NumCgoCall()
	ourMiddle := atomic.LoadInt64(&ourCount)
	close(proceed)
	final.Wait()
	time.Sleep(10 * time.Millisecond)
	cgoEnd := runtime.NumCgoCall()
	ourEnd := atomic.LoadInt64(&ourCount)

	fmt.Printf("our count of calls %d %d %d\n", ourStart, ourMiddle, ourEnd)
	fmt.Printf("runtime.NumCgoCall %d %d %d\n", cgoStart, cgoMiddle, cgoEnd)
}
@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jun 16, 2021
@ianlancetaylor ianlancetaylor added this to the Go1.18 milestone Jun 16, 2021
@smasher164
Copy link
Member

Is there a reason why ncgocall is stored per M, and isn't a monotonically increasing global counter?

qingyunha added a commit to qingyunha/go that referenced this issue Jun 21, 2021
ncgocall was stored per M, runtime.NumCgoCall lost the counter when a M die.

Fixes golang#46789
@gopherbot
Copy link

Change https://golang.org/cl/329729 mentions this issue: runtime: make ncgocall a global counter

qingyunha added a commit to qingyunha/go that referenced this issue Jun 21, 2021
ncgocall was stored per M, runtime.NumCgoCall lost the counter when a M die.

Fixes golang#46789
qingyunha added a commit to qingyunha/go that referenced this issue Jun 22, 2021
ncgocall was stored per M, runtime.NumCgoCall lost the counter when a M die.

Fixes golang#46789
@golang golang locked and limited conversation to collaborators Jun 25, 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

Successfully merging a pull request may close this issue.

4 participants