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

Jump to bottom

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
Closed

runtime: non-cooperative goroutine preemption does not work when built using c-archive or c-shared #49288

wangfakang opened this issue Nov 2, 2021 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.18

Comments

@wangfakang
Copy link

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 The path to resolution is known, but the work has not been done. 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

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

@wangfakang
Copy link
Author

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

@ianlancetaylor
Copy link
Contributor

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 commented Nov 16, 2021 •
edited

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?

go/src/runtime/sys_linux_amd64.s

Lines 436 to 440 in 6b3f4d3

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

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.

@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
3 participants
@ianlancetaylor @wangfakang @gopherbot