Merge pull request #37 from xianml/fix/issue-30-signal-handlers

fix(#30): preserve Go's signal handlers across chdb_connect; add race detector to CI

Author: wudidapaopao

.github/workflows/chdb.yml

@@ -28,6 +28,8 @@ jobs:
           make build
     - name: Test
       run: make test
+    - name: Test with race detector
+      run: go test -race -timeout=180s ./...
     - name: Test main
       run: ./chdb-go "SELECT 12345"
 
@@ -48,6 +50,8 @@ jobs:
           make build
     - name: Test
       run: make test
+    - name: Test with race detector
+      run: go test -race -timeout=180s ./...
     - name: Test main
       run: ./chdb-go "SELECT 12345"
 

chdb-purego/binding.go

@@ -3,11 +3,75 @@ package chdbpurego
 import (
 	"os"
 	"os/exec"
+	"syscall"
 	"unsafe"
 
 	"github.com/ebitengine/purego"
 )
 
+// sigactionBufSize is large enough to hold a struct sigaction on every
+// platform we care about: 16 B on macOS, ~152 B on glibc/Linux.
+const sigactionBufSize = 256
+
+// signalsToProtect lists the signals the Go runtime owns and must keep
+// owning. libchdb's chdb_set_signal_handlers_enabled(0) wipes the kernel's
+// handler list for the same set as a side effect; we save and restore these
+// around that call so Go's handlers survive.
+//
+// Signal numbers are NOT the same on Linux and macOS (e.g. SIGBUS is 10 on
+// Darwin but 7 on Linux; SIGURG is 16 on Darwin but 23 on Linux). Use the
+// syscall package's per-platform constants so the values resolve correctly
+// at compile time on each OS.
+var signalsToProtect = []int{
+	int(syscall.SIGILL),
+	int(syscall.SIGABRT),
+	int(syscall.SIGFPE),
+	int(syscall.SIGBUS),
+	int(syscall.SIGSEGV),
+	int(syscall.SIGURG), // Go uses SIGURG for async preemption
+}
+
+// libcSigaction is the libc sigaction(2) function, resolved from whichever
+// libc the process was already linked against. We pass opaque buffers
+// rather than typed structs so the same code works for the differently-
+// laid-out struct sigaction on macOS vs glibc.
+var libcSigaction func(sig int, act, oact unsafe.Pointer) int
+
+func loadSigaction() {
+	// Prefer the empty-path form: on Linux (glibc, musl) dlopen("") returns
+	// a handle to the running process's symbol table, which always contains
+	// sigaction because libc is already loaded by the Go runtime. No path
+	// to maintain.
+	libc, err := purego.Dlopen("", purego.RTLD_NOW)
+	if err != nil {
+		// macOS dyld interprets "" as a file lookup, not as "current
+		// process", so the empty-path form fails. Fall back to libSystem,
+		// which is always present and always contains sigaction.
+		libc, err = purego.Dlopen("/usr/lib/libSystem.B.dylib", purego.RTLD_NOW)
+	}
+	if err != nil {
+		panic("chdb-purego: cannot resolve sigaction(2): " + err.Error())
+	}
+	purego.RegisterLibFunc(&libcSigaction, libc, "sigaction")
+}
+
+// snapshotSignalHandlers stores the current sigaction state for the signals
+// we need to protect, in opaque buffers.
+func snapshotSignalHandlers() [][sigactionBufSize]byte {
+	saved := make([][sigactionBufSize]byte, len(signalsToProtect))
+	for i, sig := range signalsToProtect {
+		libcSigaction(sig, nil, unsafe.Pointer(&saved[i][0]))
+	}
+	return saved
+}
+
+// restoreSignalHandlers writes a previously-snapshotted sigaction state back.
+func restoreSignalHandlers(saved [][sigactionBufSize]byte) {
+	for i, sig := range signalsToProtect {
+		libcSigaction(sig, unsafe.Pointer(&saved[i][0]), nil)
+	}
+}
+
 func findLibrary() string {
 	// Env var
 	if envPath := os.Getenv("CHDB_LIB_PATH"); envPath != "" {
@@ -66,6 +130,13 @@ var (
 	chdbResultStorageRowsRead  func(result *chdb_result) uint64
 	chdbResultStorageBytesRead func(result *chdb_result) uint64
 	chdbResultError            func(result *chdb_result) string
+
+	// Process-wide signal handler control. See issue #30: by default libchdb
+	// installs its own SIGSEGV/SIGABRT/SIGBUS/SIGILL handlers when a
+	// connection is opened, which overwrites the Go runtime's handlers and
+	// breaks async preemption (SIGURG) and stack-growth (SIGSEGV) handling.
+	chdbSetSignalHandlersEnabled func(enabled int)
+	chdbResetSignalHandlers      func()
 )
 
 func init() {
@@ -105,4 +176,59 @@ func init() {
 	purego.RegisterLibFunc(&chdbResultStorageBytesRead, libchdb, "chdb_result_storage_bytes_read")
 	purego.RegisterLibFunc(&chdbResultError, libchdb, "chdb_result_error")
 
+	// Signal handler protection (issue #30). The required API was added in
+	// libchdb v26.x via chdb-core#11. Pre-check the symbol with Dlsym so
+	// that older libchdb builds — which don't export
+	// chdb_set_signal_handlers_enabled — degrade gracefully instead of
+	// panicking inside RegisterLibFunc at init. On those builds the crash
+	// from issue #30 stays unfixed, but the binding still loads.
+	if sym, dlsymErr := purego.Dlsym(libchdb, "chdb_set_signal_handlers_enabled"); dlsymErr == nil && sym != 0 {
+		purego.RegisterLibFunc(&chdbSetSignalHandlersEnabled, libchdb, "chdb_set_signal_handlers_enabled")
+		purego.RegisterLibFunc(&chdbResetSignalHandlers, libchdb, "chdb_reset_signal_handlers")
+
+		// Tell libchdb NOT to install its own signal handlers on the
+		// first chdb_connect(). Without this, libchdb's ClickHouse
+		// daemon code installs handlers for SIGSEGV / SIGABRT / SIGBUS
+		// / SIGILL / SIGFPE the first time a connection is opened,
+		// overwriting the handlers the Go runtime relies on for stack
+		// growth and panic recovery. When a Go signal then lands in
+		// libchdb's handler instead of the runtime's, the result is
+		// the rare std::mutex::unlock crash described in issue #30 —
+		// most often on macOS arm64 CI where signal pressure is
+		// highest.
+		//
+		// Important: chdb_set_signal_handlers_enabled(0) doesn't only
+		// set a flag — it also wipes the existing handlers in the same
+		// set back to SIG_DFL (verified empirically; the chdb.h header
+		// doesn't mention this side effect). Since the Go runtime has
+		// already installed its handlers by the time this package's
+		// init runs, we have to save Go's handlers, call the disable,
+		// then restore them. After this dance the libchdb flag is set
+		// (so no future connect installs handlers) and Go's handlers
+		// remain in place.
+		loadSigaction()
+		func() {
+			defer guardSignalHandlers()()
+			chdbSetSignalHandlersEnabled(0)
+		}()
+	}
+}
+
+// signalProtectionAvailable reports whether the running libchdb exposes the
+// signal handler control API. When false, snapshotSignalHandlers and
+// restoreSignalHandlers must NOT be called (libcSigaction was not loaded).
+func signalProtectionAvailable() bool {
+	return chdbSetSignalHandlersEnabled != nil
+}
+
+// guardSignalHandlers returns a function suitable for `defer`-ing around a
+// call into libchdb. When signal protection is available, it captures the
+// current sigaction state now and the returned closure restores it. When
+// not available (old libchdb), both calls are no-ops.
+func guardSignalHandlers() func() {
+	if !signalProtectionAvailable() {
+		return func() {}
+	}
+	saved := snapshotSignalHandlers()
+	return func() { restoreSignalHandlers(saved) }
 }

chdb-purego/chdb.go

@@ -217,6 +217,23 @@ func NewConnection(argc int, argv []string) (ChdbConn, error) {
 	var conn *chdb_connection
 	var err error
 	func() {
+		// chdb_connect — even after chdb_set_signal_handlers_enabled(0) —
+		// still resets the kernel sigaction table for SIGSEGV / SIGABRT /
+		// SIGBUS / SIGILL / SIGFPE to SIG_DFL on the first call (verified
+		// empirically; see issue #30 for context). guardSignalHandlers
+		// snapshots Go's handlers now and the deferred closure restores
+		// them on return — even if chdb_connect panics (a C++ exception
+		// propagated through purego), the LIFO defer order runs
+		// recover() first to set err and then the restore, leaving the
+		// process with Go's stack-growth / panic-recovery handlers
+		// intact.
+		//
+		// On old libchdb builds that don't export the signal-handler
+		// control API, guardSignalHandlers returns a no-op closure and
+		// the call site behaves exactly as it did before the issue #30
+		// fix landed.
+		defer guardSignalHandlers()()
+
 		defer func() {
 			if r := recover(); r != nil {
 				err = fmt.Errorf("C++ exception: %v", r)

chdb-purego/sigstress_test.go

@@ -0,0 +1,118 @@
+package chdbpurego
+
+import (
+	"os"
+	"runtime"
+	"strconv"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+)
+
+// TestSignalRepro is a deliberately adversarial stress test aimed at
+// reproducing the crash from issue #30 locally. Skipped unless STRESS=1.
+//
+// Strategy: mimic GitHub Actions macos-14 conditions as closely as possible:
+//   - run with `GOMAXPROCS=3` (the GHA runner has 3 cores allocated);
+//   - launch many more goroutines than P (heavy oversubscription → Go runtime
+//     fires SIGURG aggressively for async preemption);
+//   - recurse with large stack frames so the runtime needs to grow stacks
+//     (SIGSEGV on the guard page) frequently;
+//   - drive a chdb query on every iteration so libchdb has live work in
+//     progress whenever a Go runtime signal lands.
+//
+// Knobs (env):
+//
+//	STRESS_DURATION  = duration string (default 60s)
+//	STRESS_WORKERS   = explicit worker count (default 16 * GOMAXPROCS)
+//	STRESS_FRAME_KB  = per-frame allocation size in KB (default 8)
+//	STRESS_DEPTH     = recursion depth before issuing the query (default 32)
+func TestSignalRepro(t *testing.T) {
+	if os.Getenv("STRESS") != "1" {
+		t.Skip("set STRESS=1 to run the adversarial repro")
+	}
+
+	duration := envDuration("STRESS_DURATION", 60*time.Second)
+	workers := envInt("STRESS_WORKERS", 16*runtime.GOMAXPROCS(0))
+	frameKB := envInt("STRESS_FRAME_KB", 8)
+	depth := envInt("STRESS_DEPTH", 32)
+
+	t.Logf("GOMAXPROCS=%d workers=%d frame=%dKB depth=%d duration=%s",
+		runtime.GOMAXPROCS(0), workers, frameKB, depth, duration)
+
+	conn, err := NewConnectionFromConnString(":memory:")
+	if err != nil {
+		t.Fatalf("connect: %v", err)
+	}
+	defer conn.Close()
+
+	var (
+		wg       sync.WaitGroup
+		queries  atomic.Uint64
+		failures atomic.Uint64
+		stop     atomic.Bool
+	)
+
+	for i := 0; i < workers; i++ {
+		wg.Add(1)
+		go func(id int) {
+			defer wg.Done()
+			growAndQuery(conn, depth, frameKB, &queries, &failures, &stop)
+		}(i)
+	}
+
+	time.Sleep(duration)
+	stop.Store(true)
+	wg.Wait()
+
+	t.Logf("queries=%d failures=%d (%.0f qps)",
+		queries.Load(), failures.Load(),
+		float64(queries.Load())/duration.Seconds())
+	if failures.Load() != 0 {
+		t.Fatalf("%d query failures under repro stress", failures.Load())
+	}
+}
+
+// growAndQuery recurses with a large stack frame to force Go stack growth via
+// SIGSEGV on the guard page, then drives queries in a tight loop. Each leaf
+// goroutine alternates work between Go and libchdb-side C++, maximising the
+// chance that a runtime signal lands inside libchdb.
+func growAndQuery(conn ChdbConn, depth, frameKB int, queries, failures *atomic.Uint64, stop *atomic.Bool) {
+	if depth > 0 {
+		// allocate a sizable local so the Go stack must grow at this frame
+		frame := make([]byte, frameKB*1024)
+		_ = frame[0]
+		growAndQuery(conn, depth-1, frameKB, queries, failures, stop)
+		return
+	}
+	for !stop.Load() {
+		res, err := conn.Query("SELECT 1 + 1", "CSV")
+		if err != nil {
+			failures.Add(1)
+			continue
+		}
+		if res != nil {
+			res.Free()
+		}
+		queries.Add(1)
+	}
+}
+
+func envInt(key string, def int) int {
+	if v := os.Getenv(key); v != "" {
+		if n, err := strconv.Atoi(v); err == nil {
+			return n
+		}
+	}
+	return def
+}
+
+func envDuration(key string, def time.Duration) time.Duration {
+	if v := os.Getenv(key); v != "" {
+		if d, err := time.ParseDuration(v); err == nil {
+			return d
+		}
+	}
+	return def
+}