CC @aclements @prattmic

CC @cherrymui

This appears to have been caused by github.com/benesch/cgosymbolizer, removing that import solves the issue. We'll come up with a test case for this, although it may well be a bug in that project.

Here's a simple reproduction that continuously allocates and frees 1 GB of memory in two goroutines, and outputs the goroutine stacks every 30 seconds. Both goroutines will eventually block, the first one within about 20 seconds.

package main

import (
	// #include <stdlib.h>
	"C"
	"fmt"
	"math"
	"runtime"
	"time"
	"unsafe"

	_ "github.com/benesch/cgosymbolizer"
)

const (
	concurrency = 2
	size        = 32768
	count       = int(1e9) / size // 1 GB per goroutine
)

func main() {
	for w := 0; w < concurrency; w++ {
		go func(w int) {
			for i := 0; ; i++ {
				fmt.Printf("goroutine:%d cycles:%d\n", w, i)

				allocs := make([][]byte, 0, count)
				for i := 0; i < count; i++ {
					allocs = append(allocs, alloc(size))
				}
				for _, a := range allocs {
					free(a)
				}
			}
		}(w)
	}

	buf := make([]byte, 65536)
	for {
		time.Sleep(30 * time.Second)
		n := runtime.Stack(buf, true)
		fmt.Printf("%s\n", buf[:n])
	}
}

func alloc(n int) []byte {
	ptr := C.calloc(C.size_t(n), 1)
	if ptr == nil {
		panic("out of memory")
	}
	return (*[math.MaxInt32]byte)(ptr)[:n:n]
}

func free(b []byte) {
	C.free(unsafe.Pointer(&b[0]))
}

Here's an even smaller test case. Interestingly, this will have hung goroutines even with GODEBUG=asyncpreemptoff=1. Although the failure mode here may be different, as the stack traces don't show timing information for the syscalls.

package main

import (
	// #include <stdlib.h>
	"C"
	"fmt"
	"os"
	"runtime"
	"syscall"
	"time"

	_ "github.com/benesch/cgosymbolizer"
)

const concurrency = 2

func main() {
	envName := C.CString("GODEBUG")
	for w := 0; w < concurrency; w++ {
		go func(w int) {
			for i := 0; ; i++ {
				if i%1e6 == 0 {
					fmt.Printf("goroutine:%d calls:%d\n", w, i)
					runtime.Gosched()
				}
				C.getenv(envName)
			}
		}(w)
	}
	go func() {
		p, _ := os.FindProcess(os.Getpid())
		for {
			p.Signal(syscall.SIGUSR1)
			time.Sleep(time.Millisecond)
		}
	}()
	buf := make([]byte, 65536)
	for {
		time.Sleep(30 * time.Second)
		n := runtime.Stack(buf, true)
		fmt.Printf("%s\n", buf[:n])
	}
}

Now I'm curious whether github.com/ianlancetaylor/cgosymbolizer works any better.

Now I'm curious whether github.com/ianlancetaylor/cgosymbolizer works any better.

Gave that a try too, same behavior.

In the original issue report, the deadlock is happening because the program is trying to fetch a stack trace while calling the C function calloc. That can happen if a signal is received during the call to calloc. That will cause a deadlock if the cgosymbolizer code itsef calls calloc or malloc. The github.com/benesch/cgosymbolizer code uses macOS libunwind, and at least some versions of macOS libunwind call malloc. So it's not surprising that a deadlock can occur.

The github.com/ianlancetaylor/cgosymbolizer package, on the other hand, does not call malloc. So that must be failing in a different way.

In any case it's not clear to me that this is a bug in the Go toolchain, or that there is anything that the Go project can change to fix this.

You might be able to fix the problem by changing the file cgosymbolizer_darwin.c, the function cgo_traceback, to simply return if arg->sig_ctx != 0.

Irrespective of this bug, @erikgrinaker, Cockroach should definitely switch to using Ian’s cgosymbolizer if it supports Mach-O now.

Oh, I see, you’re removing it entirely via cockroachdb/cockroach#63737. That seems reasonable.

Thank you for the explanation @ianlancetaylor, that makes a lot of sense. We did see libunwind come up when looking at the hung threads in a debugger, which fits well with what you're saying.

You might be able to fix the problem by changing the file cgosymbolizer_darwin.c, the function cgo_traceback, to simply return if arg->sig_ctx != 0.

This does indeed solve the issue, as one would expect.

In any case it's not clear to me that this is a bug in the Go toolchain, or that there is anything that the Go project can change to fix this.

I agree, nothing here indicates a problem in Go, so I'll close the issue. Thanks for having a look!

Cockroach should definitely switch to using Ian’s cgosymbolizer if it supports Mach-O now.

We don't really have a big need for this now that we no longer use RocksDB, but otherwise that might have been worth exploring. Thanks for the suggestion!

Something is still unexplained here, though. If the issue was that benesch/cgosymbolizer can sometimes call malloc, then why didn’t switching to ianlancetaylor/cgosymbolizer, which supposedly does not call malloc, fix the issue?

Something is still unexplained here, though. If the issue was that benesch/cgosymbolizer can sometimes call malloc, then why didn’t switching to ianlancetaylor/cgosymbolizer, which supposedly does not call malloc, fix the issue?

That's true. I had a look, and it's blocked on _Unwind_Backtrace (from unwind.h):

https://github.com/ianlancetaylor/cgosymbolizer/blob/7cb7081f6b86962f7f0b974699a93a5ab1c6a1c1/traceback.c#L82

 0  0x00007fff2a5b18b0 in ???
    at ?:-1
 1  0x00007fff2a5b393c in ???
    at ?:-1
 2  0x00007fff2a5b5eba in ???
    at ?:-1
 3  0x00000000040ba861 in C.cgoTraceback
    at /Users/erik/go/pkg/mod/github.com/ianlancetaylor/cgosymbolizer@v0.0.0-20210303021718-7cb7081f6b86/traceback.c:82
 4  0x00000000040ab7b9 in C.x_cgo_callers
    at /usr/local/Cellar/go/1.16.3/libexec/src/runtime/cgo/gcc_traceback.c:22

On macOS, unwind.h is an Apple/LLVM-provided implementation that was introduced with Big Sur according to this comment in unwind.h:

#if defined(__APPLE__) && __has_include_next(<unwind.h>)
/* Darwin (from 11.x on) provide an unwind.h. If that's available,
 * use it. libunwind wraps some of its definitions in #ifdef _GNU_SOURCE,
 * so define that around the include.*/
 *

I think that ends up being the same implementation as libunwind.h (also provided via LLVM), which we think calls malloc, thus causing a deadlock. Either that, or I assume the unwind implementation is not async-signal-safe in some other way. Here, too, if (arg->sigContext != 0) return; fixes the issue.

It would be extremely unfortunate if the LLVM implementation of _Unwind_Backtrace calls malloc. But I don't suppose that there is much that we can do about it.