fix(runtime): avoid reentrant weak cleanup deadlocks

runtime/internal/lib/runtime/weak_llgo.go

@@ -10,9 +10,18 @@ import (
 	psync "github.com/xgo-dev/llgo/runtime/internal/sync"
 )
 
+// weakHandle supplies the GC-dependent identity used by GOROOT's weak package.
+// Go's collector removes weak registrations from spans during sweep. BDWGC
+// invokes our cleanup during allocation, so registry removal must be deferred.
+// Removing an entry releases the runtime's reference; a user-held weak.Pointer
+// must keep its dead handle alive to preserve identity.
 type weakHandle struct {
 	key  uintptr
 	live uint32
+	// Written only by the producer before the head CAS publishes this handle;
+	// after the head Swap, only the drainer accesses the link. The head atomics
+	// order these ordinary accesses. Links retain handles, never referents.
+	next unsafe.Pointer
 }
 
 // BDWGC conservatively treats pointer-looking uintptr values as live roots.
@@ -30,13 +39,47 @@ var weakState struct {
 	once psync.Once
 	mu   psync.Mutex
 	m    map[uintptr]*weakHandle
+	dead unsafe.Pointer
 }
 
 func initWeakState() {
 	weakState.mu.Init(nil)
 	weakState.m = make(map[uintptr]*weakHandle)
 }
 
+// retireWeakHandle runs inside a GC finalizer. Even a map lookup can allocate
+// and invoke another finalizer on the same thread, so this path must neither
+// allocate nor take weakState.mu. Each handle is published exactly once.
+func retireWeakHandle(h *weakHandle) {
+	latomic.StoreUint32(&h.live, 0)
+	for {
+		head := latomic.LoadPointer(&weakState.dead)
+		h.next = head
+		if latomic.CompareAndSwapPointer(&weakState.dead, head, unsafe.Pointer(h)) {
+			return
+		}
+	}
+}
+
+// drainWeakHandles runs during registration with weakState.mu held. Detach only
+// one batch so concurrent cleanup cannot keep a registration here indefinitely.
+// This bounds the captured work, not its size: a batch of n handles takes O(n)
+// time under the registry lock. Without another non-nil weak.Make, the last
+// dead batch and its map entries remain retained; neither Value nor GC drains
+// them. Reclaiming that idle batch requires a separately scheduled safe consumer.
+func drainWeakHandles() {
+	for h := (*weakHandle)(latomic.SwapPointer(&weakState.dead, nil)); h != nil; {
+		next := (*weakHandle)(h.next)
+		// A user-held dead weak.Pointer must not retain the rest of this batch.
+		h.next = nil
+		// The address may already belong to a new object with a new handle.
+		if weakState.m[h.key] == h {
+			delete(weakState.m, h.key)
+		}
+		h = next
+	}
+}
+
 func llgoRegisterWeakPointer(p unsafe.Pointer) unsafe.Pointer {
 	if p == nil {
 		return nil
@@ -45,23 +88,19 @@ func llgoRegisterWeakPointer(p unsafe.Pointer) unsafe.Pointer {
 
 	key := encodeWeakPointer(p)
 	weakState.mu.Lock()
-	if h := weakState.m[key]; h != nil {
+	drainWeakHandles()
+	// A cleanup may mark a handle dead before publishing it to the queue.
+	if h := weakState.m[key]; h != nil && latomic.LoadUint32(&h.live) != 0 {
 		weakState.mu.Unlock()
 		return unsafe.Pointer(h)
 	}
 	h := &weakHandle{key: key, live: 1}
 	weakState.m[key] = h
 	weakState.mu.Unlock()
 
-	// Keep the cleanup closure limited to encoded identities. Capturing p here
-	// would turn the cleanup itself into a strong reference to the referent.
+	// Capture only the handle with its encoded identity, never the referent p.
 	llrt.AddCleanupPtr(p, func() {
-		latomic.StoreUint32(&h.live, 0)
-		weakState.mu.Lock()
-		if weakState.m[key] == h {
-			delete(weakState.m, key)
-		}
-		weakState.mu.Unlock()
+		retireWeakHandle(h)
 	})
 	return unsafe.Pointer(h)
 }

test/_stress/README.md

@@ -33,6 +33,7 @@ go test -race -count=3 -timeout=20m ./runtime/timer
 /tmp/llgo-runtime-stress test -count=3 -timeout=30m ./runtime/signal
 /tmp/llgo-runtime-stress test -count=3 -timeout=30m ./runtime/cpuprof
 /tmp/llgo-runtime-stress test -count=3 -timeout=30m ./runtime/finalizer
+/tmp/llgo-runtime-stress test -count=3 -timeout=5m ./runtime/weak
 ```
 
 The signal suite is LLGo-only and targets Unix hosts. The CPU profile suite is
@@ -66,3 +67,12 @@ regress fatal native-handler replacement windows. The finalizer suite
 repeatedly publishes large finalizer batches while many goroutines call
 `runtime.GC`, and checks that queued callbacks are neither corrupted nor
 delivered twice.
+
+The weak suite creates batches of weak pointers in a goroutine that exits,
+then performs bounded collections while retaining only the weak handles. It
+regresses cleanup callbacks recursively invoking another cleanup while
+allocating under the weak registry lock. Each batch must expire more than half
+its handles, and a separate helper process enforces a 60-second deadline even
+if the tested runtime deadlocks. It uses only public Go APIs and also runs with
+`go test` on Go 1.24 or newer. Use identical profiles before and after the fix;
+neither sleeping nor reducing allocation pressure is part of the workload.

test/_stress/runtime/weak/weak_stress_test.go

@@ -0,0 +1,97 @@
+//go:build go1.24 && !baremetal && !nogc && !wasm
+
+package weakstress
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"runtime"
+	"testing"
+	"time"
+	"weak"
+)
+
+type referent struct {
+	id  int
+	pad [256]byte
+}
+
+func stressCount(t *testing.T, base int) int {
+	t.Helper()
+	switch profile := os.Getenv("LLGO_STRESS_PROFILE"); profile {
+	case "", "default":
+		return base
+	case "quick":
+		return base / 8
+	case "heavy":
+		return base * 2
+	default:
+		t.Fatalf("unknown LLGO_STRESS_PROFILE %q", profile)
+		return 0
+	}
+}
+
+func TestWeakCleanupReentrancy(t *testing.T) {
+	if os.Getenv("LLGO_STRESS_WEAK_CHILD") != t.Name() {
+		// A GC callback can deadlock the allocating thread, including testing's
+		// own timeout machinery. Keep the hard deadline in a separate process.
+		executable, err := os.Executable()
+		if err != nil {
+			t.Fatal(err)
+		}
+		ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+		defer cancel()
+		cmd := exec.CommandContext(ctx, executable, "-test.run=^"+t.Name()+"$", "-test.v", "-test.timeout=45s")
+		cmd.Env = append(os.Environ(), "LLGO_STRESS_WEAK_CHILD="+t.Name())
+		output, err := cmd.CombinedOutput()
+		t.Logf("%s", output)
+		if ctx.Err() != nil {
+			t.Fatalf("weak cleanup helper did not exit within 60s: %v", ctx.Err())
+		}
+		if err != nil {
+			t.Fatalf("weak cleanup helper failed: %v", err)
+		}
+		return
+	}
+
+	rounds, objects := stressCount(t, 16), stressCount(t, 8192)
+	t.Logf("rounds=%d objects=%d GC_MARKERS=%q", rounds, objects, os.Getenv("GC_MARKERS"))
+	for round := 0; round < rounds; round++ {
+		handles := make(chan []weak.Pointer[referent], 1)
+		go func() {
+			live := make([]*referent, objects)
+			batch := make([]weak.Pointer[referent], objects)
+			for i := range live {
+				live[i] = &referent{id: i}
+				batch[i] = weak.Make(live[i])
+			}
+			handles <- batch
+			runtime.KeepAlive(live)
+			// Exit this goroutine so conservative stack scanning does not keep
+			// the entire batch alive while another goroutine collects it.
+		}()
+		batch := <-handles
+		var expired int
+		for collection := 0; collection < 16; collection++ {
+			// Finite collections avoid mixing collector/allocator starvation
+			// from an unbounded GC loop into weak-callback reentrancy coverage.
+			runtime.GC()
+			expired = 0
+			for _, h := range batch {
+				if h.Value() == nil {
+					expired++
+				}
+			}
+			if expired > objects/2 {
+				break
+			}
+			runtime.Gosched()
+		}
+		if expired <= objects/2 {
+			t.Fatalf("round %d: only %d/%d weak handles expired", round, expired, objects)
+		}
+		t.Logf("round %d: expired %d/%d weak handles", round, expired, objects)
+		runtime.KeepAlive(batch)
+	}
+}

test/std/weak/weak_test.go

@@ -128,3 +128,24 @@ func TestPointerNilInput(t *testing.T) {
 		t.Errorf("Make(nil).Value() = %v, want nil", val)
 	}
 }
+
+func TestPointerIdentity(t *testing.T) {
+	x, y := new([256]byte), new([256]byte)
+	x[0], y[0] = 1, 2
+	xw, yw := weak.Make(x), weak.Make(y)
+	if xw != weak.Make(x) {
+		t.Fatal("repeated Make returned a different handle for the same object")
+	}
+	if xw == yw {
+		t.Fatal("Make returned the same handle for distinct objects")
+	}
+	runtime.GC()
+	if xw != weak.Make(x) || yw != weak.Make(y) {
+		t.Fatal("GC changed the identity of a live weak pointer")
+	}
+	if xw.Value() != x || yw.Value() != y {
+		t.Fatal("GC invalidated a live weak pointer")
+	}
+	runtime.KeepAlive(x)
+	runtime.KeepAlive(y)
+}