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/cgo: incorrectly handle SIGPIPE(and other signals) from non-Go thread #56150

Closed
zkonge opened this issue Oct 11, 2022 · 9 comments
Closed
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@zkonge
Copy link

zkonge commented Oct 11, 2022

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

$ go version
go version go1.19.2 linux/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=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/secret/.cache/go-build"
GOENV="/home/secret/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/secret/go/pkg/mod"
GOOS="linux"
GOPATH="/home/secret/go"
GOPROXY="https://goproxy.cn|https://proxy.golang.org|direct"
GOROOT="/data00/home/secret/go1.19"
GOSUMDB="sum.golang.google.cn"
GOTMPDIR=""
GOTOOLDIR="/data00/home/secret/go1.19/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build457638227=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Following code is a minimal example. SIGPIPE raised from linux socket write, but can be reproduced by the following form.

According to the document, SIGPIPE should be caught by default, without affect program, just like the kill line in test function. But in sub-thread created by pthread, the SIGPIPE trigger the runtime.raisebadsignal, and make program exit, like kill line in subThread function (uncomment it).

package main

/*
#include <pthread.h>
#include <signal.h>
#include <unistd.h>
#include <sys/syscall.h>
#define gettid() syscall(SYS_gettid)

void *subThread(void *_)
{
    // kill(gettid(), SIGPIPE); // kill by thread id
    return NULL;
}

void test()
{
    pthread_t thread_id;
    pthread_create(&thread_id, NULL, subThread, NULL);
    pthread_join(thread_id, NULL);

    kill(gettid(), SIGPIPE);
}
*/
import "C"

func main() {
	C.test()

	// should not exit
	select {}
}

What did you expect to see?

Go runtime catch the SIGPIPE from thread created by pthread, and everything works fine.

What did you see instead?

(uncomment kill function in subThread)
kill in subThread make program exit, kill in test make program continue running.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Oct 11, 2022
@wdvxdr1123
Copy link
Contributor

cc @ianlancetaylor

@ianlancetaylor
Copy link
Contributor

The SIGPIPE rules in the documentation only apply to a SIGPIPE signal that was raised by the kernel (due to a write to a broken pipe). They do not apply to a SIGPIPE sent via the kill system call.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Oct 11, 2022
@zkonge
Copy link
Author

zkonge commented Oct 11, 2022

NOTICE that, the signal actually raised by socket write (as i said at begin). You can reproduced it with socket write broken pipe.I just want to find a equal form. With gdb i find socket broken pipe and kill SIGPIPE both trigger raisebadsignal and they have the same call stack

@zkonge
Copy link
Author

zkonge commented Oct 11, 2022

@ianlancetaylor
Hi, I write a socket broke pipe example, just like the example above, uncomment raiseSigpipe in subThread, it will make program exit.

I think it's better to reopen the issue.

package main

/*
#include <pthread.h>
#include <sys/socket.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <arpa/inet.h>
#include <sys/socket.h>

void raiseSigpipe()
{
    char fake_buf[1024];

    int sock = socket(AF_INET, SOCK_STREAM, 0);
    struct sockaddr_in sa;
    memset(&sa, 0, sizeof(sa));
    sa.sin_family = AF_INET;
    sa.sin_addr.s_addr = inet_addr("127.0.0.1");
    sa.sin_port = htons(8989);
    connect(sock, (struct sockaddr *)&sa, sizeof(sa));
    send(sock, fake_buf, 1024, 0);
    sleep(2);
    send(sock, fake_buf, 1024, 0);
    send(sock, fake_buf, 1024, 0); // sigpipe
}

void *subThread(void *_)
{
    // raiseSigpipe();
    return NULL;
}

void test()
{
    pthread_t thread_id;
    pthread_create(&thread_id, NULL, subThread, NULL);
    pthread_join(thread_id, NULL);

    raiseSigpipe();
}
*/
import "C"

import (
	"net"
)

func main() {

	go func() {
		listener, _ := net.Listen("tcp", "127.0.0.1:8989")
		for {
			var d [1024]byte
			conn, _ := listener.Accept()
			conn.(*net.TCPConn).SetLinger(0)
			conn.Read(d[:])
			conn.Close()
		}
	}()

	C.test()

	// should not exit
	println("run")
	select {}
}

@ianlancetaylor
Copy link
Contributor

My apologies, you're right: the code does check whether a SIGPIPE was delivered to a Go thread or a C thread. I'll update the docs.

@gopherbot
Copy link

Change https://go.dev/cl/442415 mentions this issue: os/signal: document behavior of SIGPIPE on non-Go thread

@zkonge
Copy link
Author

zkonge commented Oct 11, 2022

Actually, signal hander defined in go can process the SIGPIPE raised in pthread thread. Is the behavior expected according to the doc?

@zkonge
Copy link
Author

zkonge commented Oct 11, 2022

... And why not make program also ignore SIGPIPE from C thread as all signal processed by go signal hander ?

@ianlancetaylor
Copy link
Contributor

We don't know what the C code is doing with SIGPIPE. There is no choice that is always correct. The current choice seems as good as any, and since we've had it for several years we may as well keep it.

If a Go program calls C code that doesn't want to get SIGPIPE, the C code install its own handler, perhaps SIG_IGN, for SIGPIPE.

@hopehook hopehook added Documentation NeedsFix The path to resolution is known, but the work has not been done. labels Oct 12, 2022
@hopehook hopehook added this to the Backlog milestone Oct 12, 2022
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
Fixes golang#56150

Change-Id: Id990783562950ba8be7ce9526b7a811625f2190a
Reviewed-on: https://go-review.googlesource.com/c/go/+/442415
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants