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

iter: Pull doesn't work with Cgo callbacks #65946

Closed
eliasnaur opened this issue Feb 27, 2024 · 3 comments
Closed

iter: Pull doesn't work with Cgo callbacks #65946

eliasnaur opened this issue Feb 27, 2024 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@eliasnaur
Copy link
Contributor

eliasnaur commented Feb 27, 2024

Go version

go version go1.22.0 darwin/arm64 and go version devel go1.23-fc0d9a4b7d Mon Feb 26 22:45:28 2024 +0000 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/a/Library/Caches/go-build'
GOENV='/Users/a/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT='rangefunc'
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/a/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/a/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/a/Downloads/lockedpull/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/9c/t7b8z6m93sxgr4m4m4kbvllw0000gn/T/go-build3759693401=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

While experimenting with iter.Pull to see whether pull iterators may obviate the need for #64777 (and #64755), I created a program with the files

// main.go

package main

/*
void c_callbacks(void);
*/
import "C"
import (
	"fmt"
	"iter"
	"runtime"
)

var yieldThread func(struct{}) bool

//export goCallback
func goCallback() {
	yieldThread(struct{}{})
}

var iterator = func(yield func(struct{}) bool) {
	yieldThread = yield
	C.c_callbacks()
}

func main() {
	runtime.LockOSThread()
	pullIterate()
	// forIterate()
}

func forIterate() {
	for _ = range iterator {
		fmt.Println("yielded!")
	}
}

func pullIterate() {
	next, _ := iter.Pull(iterator)
	for {
		next()
		fmt.Println("yielded!")
	}
}

and

// main.c

#include "_cgo_export.h"

void c_callbacks(void) {
  for (;;) {
    goCallback();
  }
}

What did you see happen?

$ GOEXPERIMENT=rangefunc go run .
...
yielded!
yielded!
runtime:stoplockedm: lockedg (atomicstatus=4) is not Grunnable or Gscanrunnable
runtime:   gp: gp=0x14000186540, goid=19, gp->atomicstatus=4
runtime: getg:  g=0x102fedd80, goid=0,  g->atomicstatus=0
fatal error: stoplockedm: not runnable

runtime stack:
runtime.throw({0x102f37fdb?, 0x102fedd80?})
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/panic.go:1023 +0x40 fp=0x16cf5b070 sp=0x16cf5b040 pc=0x102ed8a20
runtime.stoplockedm()
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/proc.go:3031 +0x1f4 fp=0x16cf5b0d0 sp=0x16cf5b070 pc=0x102ee0e24
runtime.schedule()
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/proc.go:3847 +0x34 fp=0x16cf5b110 sp=0x16cf5b0d0 pc=0x102ee35c4
runtime.goschedImpl(0x140000021c0, 0x1)
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/proc.go:4065 +0x1e4 fp=0x16cf5b160 sp=0x16cf5b110 pc=0x102ee3ec4
runtime.gopreempt_m(...)
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/proc.go:4082
runtime.newstack()
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/stack.go:1070 +0x2cc fp=0x16cf5b310 sp=0x16cf5b160 pc=0x102ef395c
runtime.morestack()
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/asm_arm64.s:341 +0x70 fp=0x16cf5b310 sp=0x16cf5b310 pc=0x102f08120
...

If I comment out the runtime.LockOSThread I get a different error:

$ GOEXPERIMENT=rangefunc go run .
...
yielded!
yielded!
yielded!
fatal error: runtime: internal error: misuse of lockOSThread/unlockOSThread

runtime stack:
runtime.throw({0x100318601?, 0x0?})
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/panic.go:1023 +0x40 fp=0x170b8aef0 sp=0x170b8aec0 pc=0x1002b4b00
runtime.badunlockosthread()
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/proc.go:5261 +0x28 fp=0x170b8af10 sp=0x170b8aef0 pc=0x1002c2c58
runtime.systemstack(0x7fc000)
	/nix/store/sbg2nmns0b8invf6ws8yid0gdak18w4d-go-1.22.0/share/go/src/runtime/asm_arm64.s:243 +0x6c fp=0x170b8af20 sp=0x170b8af10 pc=0x1002e407c

I haven't tested the equivalent to Windows' syscall.NewCallback that behaves similar to Cgo callbacks.

What did you expect to see?

Replacing pullIterate with forIterate makes the program run as expected:

$ GOEXPERIMENT=rangefunc go run .
yielded!
yielded!
yielded!
...

This may very well be a duplicate of #65889.

@eliasnaur eliasnaur changed the title iter: Pull doesn't work with C callbacks iter: Pull doesn't work with Cgo callbacks Feb 27, 2024
@cagedmantis cagedmantis added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. compiler/runtime Issues related to the Go compiler and/or runtime. labels Feb 29, 2024
@cagedmantis
Copy link
Contributor

@golang/runtime

@cagedmantis cagedmantis added this to the Go1.23 milestone Feb 29, 2024
@mknyszek
Copy link
Contributor

I think this fix will basically go hand-in-hand with the fix to #65889.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/583675 mentions this issue: runtime: fix coro interactions with thread-locked goroutines

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. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Development

No branches or pull requests

4 participants