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: non-cooperative goroutine preemption does not work when built using c-archive or c-shared #49288

Closed
wangfakang opened this issue Nov 2, 2021 · 5 comments
Labels
NeedsFix
Milestone

Comments

@wangfakang
Copy link

@wangfakang wangfakang commented Nov 2, 2021

Non-cooperative goroutine preemption does not work when built using c-archive or c-shared, because of this PR not install signal go handler and cause signal preemption disabled.

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

$ 1.14

Does this issue reproduce with the latest release?

yeah.

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

go env Output
$ go env

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/fakang.wfk/.cache/go-build"
GOENV="/home/fakang.wfk/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"

What did you do?

test.c

#include <stdio.h>
#include <unistd.h>

#include"cgo.h"

int main(){
    sleep(1);
    printf("start cgo\n");
    test();
    printf("exit cgo\n");
    sleep(5);
    return 0;
}

cgo.go

package main

import (
        "runtime"
)

import "C"

func init() {
        runtime.GOMAXPROCS(1)
        go func() {
                println("init ok")
                for {
                }
        }()
}

//export test
func test() {
        println("I got scheduled!")
}

func main() {}

run:

go build -o cgo.so -buildmode=c-shared cgo.go
gcc -o main main.c cgo.so

What did you expect to see?

./main

init ok
start cgo
I got scheduled!
exit cgo

What did you see instead?

./main

init ok
start cgo
@ianlancetaylor ianlancetaylor changed the title Non-cooperative goroutine preemption does not work when built using c-archive or c-shared runtime: non-cooperative goroutine preemption does not work when built using c-archive or c-shared Nov 2, 2021
@ianlancetaylor ianlancetaylor added the NeedsFix label Nov 2, 2021
@ianlancetaylor ianlancetaylor self-assigned this Nov 2, 2021
@ianlancetaylor ianlancetaylor added this to the Go1.18 milestone Nov 2, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 2, 2021

Change https://golang.org/cl/360861 mentions this issue: runtime: install sigPreempt signal handler for c-archive/c-shared

@wangfakang
Copy link
Author

@wangfakang wangfakang commented Nov 12, 2021

@ianlancetaylor The signal _SIGPROF may also need to be added because CPUProfile depends on it.
image

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 12, 2021

Please always write plain text as plain text, never as an image. Plain text is much easier to read and it is possible to cut and paste it. Thanks.

If Go profiling is requested a signal handler will be installed for SIGPROF if necessary. We definitely don't want c-archive or c-shared Go code to override the SIGPROF handling of the main program; we only want to use the Go handling if the Go program specifically requests it.

@wangfakang
Copy link
Author

@wangfakang wangfakang commented Nov 16, 2021

Please always write plain text as plain text, never as an image. Plain text is much easier to read and it is possible to cut and paste it. Thanks.

If Go profiling is requested a signal handler will be installed for SIGPROF if necessary. We definitely don't want c-archive or c-shared Go code to override the SIGPROF handling of the main program; we only want to use the Go handling if the Go program specifically requests it.

@ianlancetaylor Um, sorry. Because I didn't have permission to comment directly on the go-review.googlesource.com platform, so do it.
I have a question that if we don't want c-archive or c-shared Go code to override the SIGPROF handling of the main program, but why is it necessary to check SIGPROF in runtime·cgoSigtramp?

sigtrampnog:
// Signal arrived on a non-Go thread. If this is SIGPROF, get a
// stack trace.
CMPL DI, $27 // 27 == SIGPROF
JNZ sigtramp

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 16, 2021

The cgoSigtramp function is used in all build modes, not just c-archive and c-shared. Also, to be clear, I am saying that for c-archive and c-shared we must not override the handling of SIGPROF by default. If the program itself calls Go functions like runtime/pprof.StartCPUProfile, then the Go standard library will go ahead and override the handling of SIGPROF. And in that case of course it's important that cgoSigtramp handle SIGPROF correctly.

Further discussion of these points should take place in a forum, not here on the issue tracker. See https://golang.org/wiki/Questions. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix
Projects
None yet
Development

No branches or pull requests

3 participants